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

[EuiResizeObserver, EuiBottomBar] Account for padding and border #4909

Merged
merged 4 commits into from
Jun 23, 2021

Conversation

thompsongl
Copy link
Contributor

@thompsongl thompsongl commented Jun 22, 2021

Summary

Fixes #4907, in which it was discovered that height and width from clientRect do not include padding or border (DOM box model).
This removes reliance onclientRect data and uses getBoundingClientRect instead.

Checklist

- [ ] Check against all themes for compatibility in both light and dark modes
- [ ] Checked in mobile

  • Checked in Chrome, Safari, Edge, and Firefox

- [ ] Props have proper autodocs and playground toggles
- [ ] Added documentation

- [ ] Added or updated jest tests

  • Checked for breaking changes and labeled appropriately

- [ ] Checked for accessibility including keyboard-only and screenreader modes

  • A changelog entry exists and is marked appropriately

top,
bottom,
left,
right,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to add border sizing? I noticed this with docked flyouts where we add a 2px border, that this border isn't taken into account.
Screen Shot 2021-06-22 at 14 10 19 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh hmm. I figured border would also be part of the top, left, etc. data.
I'll update to just use getBoundingClientRect() instead, which is (I think) how it used to work.

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4909/

@thompsongl thompsongl changed the title [EuiResizeObserver, EuiBottomBar] Account for padding [EuiResizeObserver, EuiBottomBar] Account for padding and border Jun 22, 2021
@thompsongl thompsongl requested a review from cchaos June 22, 2021 18:43
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4909/

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

👍 Thanks for jumping on this one so quickly. I checked the docked flyout and bottom bar usages in Chrome, Safari and Firefox. All fixed!

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4909/

@thompsongl thompsongl merged commit 49c19ed into elastic:master Jun 23, 2021
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.

[EuiBottomBar] No longer getting correct dimensions of itself when adding padding to the body
3 participants