Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] fix(OverflowList): improve spacer width check #5168

Merged
merged 1 commit into from
Mar 16, 2022

Conversation

dylanrcooke
Copy link
Contributor

Fixes #5167

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

Fix the OverflowList spacer width check to account for css scaling

Reviewers should focus on:

Any risks to using offsetWidth instead of getBoundingClientRect()

Screenshot

No user facing change

@palantirtech
Copy link
Member

Thanks for your interest in palantir/blueprint, @dylanrcooke! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

Copy link
Member

@invliD invliD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adidahiya to confirm that this minor break is ok. We'll also need this on 4.x.

@@ -151,7 +151,7 @@ export class OverflowList<T> extends React.Component<OverflowListProps<T>, IOver
/** A cache containing the widths of all elements being observed to detect growing/shrinking */
private previousWidths = new Map<Element, number>();

private spacer: Element | null = null;
private spacer: HTMLElement | null = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically this no longer allows non-HTML usage of the overflow list, but given that it's relatively specific for HTML layout, I'm ok with this.

} else if (this.spacer.getBoundingClientRect().width < 0.9) {
} else if (this.spacer.offsetWidth < 0.9) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this works in our internal repro.

@adidahiya adidahiya changed the title Fix the OverflowList spacer width check to account for css scaling [core] fix(OverflowList): improve spacer width check to account for CSS scaling Mar 16, 2022
@adidahiya adidahiya changed the title [core] fix(OverflowList): improve spacer width check to account for CSS scaling [core] fix(OverflowList): improve spacer width check Mar 16, 2022
@adidahiya adidahiya merged commit 0336971 into palantir:develop Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overflow list does not detect popover size correctly due to scale
4 participants