[High Contrast Mode] Horizontal rules, side navs, tabs, and breadcrumbs #8218
Conversation
- this one I'm going to opt to just always show a border - it's not as potentially tricky as `<hr>` that tends to have native browser styling, and renders the same in non-high-contrast views
…x Windows rendering - opt for pseudo element over box-shadow even in default contrast - it's easier and more targeted to set `border-color` over the entire repeated box shadow position
…n favor of flex gap
- switch to border over background for win high contrast rendering - fix various token usages
- reduce className selector with `:is()` - remove "default" clip path and unsetting said default with a specific key/modifier for intermediate children (will be using this shortly for high contrast styles as well)
- reduce selector nesting - remove new drop shadow filter and make the focused item sit above the rest
|
Preview staging links for this PR:
|
💚 Build Succeeded
|
| `filter: drop-shadow(${euiTheme.border.width.thin} 0 0 ${ | ||
| highContrastMode === 'forced' ? euiTheme.border.color : borderColor | ||
| });`; | ||
| const applicationHighContrastArrow = highContrastModeStyles(euiThemeContext, { |
There was a problem hiding this comment.
Non blocking suggestion: We might be able to clean up the corners a bit by adjusting the clip-path for high contrast. 🤔
It might not be worth the override though.
// regular
clip-path: polygon(
0 0,
calc(100% - ${euiTheme.size.s}) 0,
100% 50%,
calc(100% - ${euiTheme.size.s}) 100%,
0 100%,
${euiTheme.size.s} 50%
);
// adjust position of arrow point by 1px (effectively making the gap bigger but by that also preventing the clip-path to include another items drop-shadow)
const additionalOffset = euiTheme.border.width.thin;
const offset = mathWithUnits([euiTheme.size.s, additionalOffset], (x, y) => x - y);
clip-path: polygon(
0 0,
calc(100% - ${offset}) 0,
100% 50%,
calc(100% - ${additionalOffset}) 100%,
0 100%,
${euiTheme.size.s} 50%
);current
suggested
There was a problem hiding this comment.
Haha dangit that's going to bug me forever now. Buuut I think I'm just going to ship it with the imperfection, the drop-shadow filter workaround is also slightly off on Windows high contrast (since it uses the full/empty shade and can't inherit the theme's border color) so I don't really want to add a bunch of code to try and fix barely perceptible/minor issues.
Honestly I'd prefer we just move away from this style of breadcrumbs. It sounds like we may be doing with the new serverless nav eventually, as those use non-application type breadcrumbs, so at some point this display style will get deprecated and we won't need to worry about this 🤞
There was a problem hiding this comment.
Oh I forgot to also comment - super impressive CSS-fu figuring out the fix though!! 👏
There was a problem hiding this comment.
Honestly I'd prefer we just move away from this style of breadcrumbs. It sounds like we may be doing with the new serverless nav eventually, as those use non-application type breadcrumbs, so at some point this display style will get deprecated and we won't need to worry about this.
That's a fair point. I agree that it's not needed to optimize now if it potentially becomes obsolete in the future. And if not, we can still update as needed when needed. 👍
Summary
Note
This PR merges into a feature branch
Breadcrumbs (specifically the EuiHeader/application styles) were mostly the headache here, as evidenced by the git history 🥲 I don't love the solution I went with and it has some minute color/pixel issues, but IMO it strikes a decent enough balance between providing sufficient visual contrast and maintaining headache-inducing CSS.
QA
General checklist
and screenreader modes- [ ] Checked in mobile- [ ] Added or updated jest and cypress tests