[EuiCollapsibleNavBeta] Add global CSS variable for width offset #7248
Conversation
- a unit test can't yet be meaningfully written for this (jsdom doesn't have the concept of `:root` or CSS variables), so this will have to do for now
|
Preview staging links for this PR:
|
💚 Build Succeeded
|
|
@nchaulet Feel free to reference this PR when updating yours. Usage should be as simple as Check it out: https://eui.elastic.co/pr_7248/storybook/index.html?path=/story/euicollapsiblenavbeta--global-css-variable |
| useEffect(() => { | ||
| setGlobalCSSVariables({ | ||
| '--euiCollapsibleNavOffset': isOverlay ? '0' : width, | ||
| }); | ||
| }, [width, isOverlay, setGlobalCSSVariables]); |
There was a problem hiding this comment.
This is a very clever fix. I actually didn't know we had a styling function to globally set variables.
| @@ -469,6 +470,24 @@ export const FlyoutInFixedHeaders: Story = { | |||
| }, | |||
| }; | |||
|
|
|||
| export const GlobalCSSVariable: Story = { | |||
| render: ({ ...args }) => ( | |||
There was a problem hiding this comment.
Is args being set to anything here? Is it just the component defaults? If not, could we get away without passing anything to the render function?
There was a problem hiding this comment.
We always need to pass ...args (if there's a control panel), as that's where the control values come from
| @@ -469,6 +470,24 @@ export const FlyoutInFixedHeaders: Story = { | |||
| }, | |||
| }; | |||
|
|
|||
| export const GlobalCSSVariable: Story = { | |||
| render: ({ ...args }) => ( | |||
There was a problem hiding this comment.
What do you think about hiding initialIsCollapsed and onCollapseToggle from this story since it's mainly to show how the bottom bar is affected by the width of the nav?
There was a problem hiding this comment.
@breehall Would you be cool with me making that change in #7245 as opposed to this PR? I'd like to do a release with this work so that I can get it into the currently open EUI upgrade and @nchaulet can update his Kibana PR sooner rather than later.
There's a bunch of other stories under this file that need a closer inspection of their props pane in any case, and I'd rather have the new util I just set up while doing so
There was a problem hiding this comment.
Sweet! Will update that PR in a second here!
`v88.5.0`⏩`v88.5.4` This EUI upgrade helps unblock the Shared UX team with some beta serverless nav updates not listed in the below changelog (elastic/eui#7228 and elastic/eui#7248). --- ## [`88.5.4`](https://github.com/elastic/eui/tree/v88.5.4) - This release contains internal changes to a beta component needed by Kibana. ## [`88.5.3`](https://github.com/elastic/eui/tree/v88.5.3) **Bug fixes** - Fixed `EuiComboBox` search input width not resetting correctly on selection ([#7240](elastic/eui#7240)) ## [`88.5.2`](https://github.com/elastic/eui/tree/v88.5.2) **Bug fixes** - Fixed broken `EuiTextTruncate` testenv mocks ([#7234](elastic/eui#7234)) ## [`88.5.1`](https://github.com/elastic/eui/tree/v88.5.1) - Improved the performance of `EuiComboBox` by removing the `react-autosizer-input` dependency ([#7215](elastic/eui#7215)) **Dependency updates** - Updated `react-element-to-jsx-string` to v5.0.0 ([#7214](elastic/eui#7214)) - Removed unused `@types/vfile-message` dependency ([#7214](elastic/eui#7214))
Summary
Adding this feature for elastic/kibana#166840 and so Kibana devs don't have to use hard-coded width values for portalled/fixed position content.
QA
:rootselector and uncheck the--euiCollapsibleNavOffsetvariable definition - the bottom bar should correctly fall back to a left position of0General checklist
- [ ] Checked for accessibility including keyboard-only and screenreader modes- [ ] Checked in both light and dark modes- [ ] Added or updated jest and cypress testsJest tests don't apply here, although a Cypress test could be added - but I'm opting for a Storybook for now, since component is still in beta