[Enterprise Search] Fix Kibana beta notification nav CSS overrides leaking into other plugins#107973
Conversation
- Because we were previously not scoping our Kibana CSS overrides, loading the Enterprise Search plugin and then another plugin would cause that CSS to leak
cee-chen
added
bug
| justify-content: space-between; | ||
| .betaSidebarNotification { | ||
| .euiSideNav { | ||
| display: flex; |
There was a problem hiding this comment.
This is the same CSS properties as before, just indented. I recommend hiding whitespace changes on the diff.
I changed this from .kbnPageTemplateSolutionNav to .euiSideNav for clarity, we could hook into either class but it made the most sense for me to use .euiSideNav to match .euiSideNav__content later.
There was a problem hiding this comment.
Just FYI, it's never really a good idea to target .eui classes, because we make changes these a bunch including the DOM structure. We don't usually consider them breaking changes since it's not a way we support custom styling.
It may be better to use absolute positioning in this case, or to request a customization option from the component itself. Even could do it in KibanaSolutionNav, where it just accepts children placed after the EuiSideNav component.
There was a problem hiding this comment.
I used absolute positioning originally (#104763), but that broke when the PR to collapse the sidebar landed. Definitely understood that it's not ideal to target EUI classes and we normally don't, but in this case since this is a temporary notification and will be removed in 7.16, I'm honestly not super worried about it and I don't think it needs to be semantically perfect since it has a specific (and fairly short) end of life. We'll definitely keep an eye out for any layout changes or issues that come up between now and 7.16!
There was a problem hiding this comment.
Yeah, no problem. Mainly knowledge sharing. Also probably a good idea to put in a specific GH issue for removing it (if one doesn't exist yet) and targeting 7.16.
There was a problem hiding this comment.
Awesome call, thanks Caroline! I've opened up a draft PR for 7.16 (#108085) as well as creating an issue in our team repo (https://github.com/elastic/app-search-team/issues/1986).
|
@elasticmachine run elasticsearch-ci/docs |
- Because we were previously not scoping our Kibana CSS overrides, loading the Enterprise Search plugin and then another plugin would cause that CSS to leak
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…07983) - Because we were previously not scoping our Kibana CSS overrides, loading the Enterprise Search plugin and then another plugin would cause that CSS to leak Co-authored-by: Constance <constancecchen@users.noreply.github.com>
cee-chen
added
v7.14.1
auto-backport
- Because we were previously not scoping our Kibana CSS overrides, loading the Enterprise Search plugin and then another plugin would cause that CSS to leak
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…08233) - Because we were previously not scoping our Kibana CSS overrides, loading the Enterprise Search plugin and then another plugin would cause that CSS to leak Co-authored-by: Constance <constancecchen@users.noreply.github.com>
5 participants
Summary
beta.scss,beta.tsx, and now line 82 & 66 ofshared/layout/page_template.tsx.Before
After
Checklist