-
Notifications
You must be signed in to change notification settings - Fork 861
[Visual Refresh] Revert floating border usage back to using pseudo elements #9080
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
[Visual Refresh] Revert floating border usage back to using pseudo elements #9080
Conversation
- we rather keep what we're already using and adjust the edge cases of shadow + scroll
& + & don't work due to dynamic classes which results in & not being the same; we use the global class instead as we know it's applied
acstll
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of small things, I followed the QA steps and found one single thing to double-check. Everything else working as expected, AND pretty much in favor of this update to wrap up the shadow tokens effort. Thanks again!
packages/website/docs/getting-started/theming/utilities/shadows/index.mdx
Show resolved
Hide resolved
💚 Build SucceededHistory
cc @mgadewoll |
💚 Build Succeeded
History
cc @mgadewoll |
acstll
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟢 Thanks for the updates!
Summary
Important
This PR merged into a feature branch: https://github.com/elastic/eui/tree/eui-theme/borealis-v2-poc
This PR is a follow-up to #8962 and it essentially reverts how floating borders are applied back to using a pseudo elements with
border.Context
In production, floating borders are currently applied via pseudo elements. The main reason for this was to prevent dimension shifts due to the
borderproperty. Alternatives - e.g. using transparent borders in LIGHT mode - do not fit our use cases as a transparent border would create visible gaps if the content background doesn't match the page background. (see this previous PR)#8962 changed this to use the
borderproperty directly on the element instead of a pseudo element. The reason was that the pseudo element would be volatile to scroll behavior as it's content, not container (see a more in depth comparison here).Changes
After additional consideration, it seems that the initial decision to exclude the pseudo element approach as valid option based on the scroll behavior issues was too hasty. Instead of changing how the borders are applied and accepting dimension shifts, we can rather adjust the scroll behavior issues which are less common (e.g. so far 2 usage were noticed in Kibana).
The issue that the floating border pseudo element is scrolled as content can be worked around by accepting the following guidance: We do not combine shadow utils and scroll behavior on the same element. Instead scroll behavior should be added on the child of the element that has the shadow util styling. This way both behaviors work as expected. The downside is an additional nesting level (which is only needed if both stylings are indeed needed and is not added by default).
euiFloatingBorderStylesfor panel stylesWhy are we making this change?
💅 UI consistency: Reverting back to the currently used floating border approach reduces the update risk impact and ensures we don't have dimension shifts between component variants and color modes.
Screenshots #
Impact to users
Updates for Kibana scrollable popovers
QA
General checklist
@defaultif default values are missing) and playground toggles