[EuiHeader] Update to set a new global `--euiFixedHeadersOffset` CSS variable #7144

cee-chen · 2023-08-30T02:19:04Z

Summary

This logic allows EuiHeader to afford for any number of fixed headers (although it's likely that consumers will not use more than two). The most important aspect of this refactor that other EUI components can access this CSS variable globally with no extra JS logic or rerendering on their end, which removes a significant amount of duplicated useEffects from components affected by fixed headers (EuiPageTemplate, EuiSideBar, and EuiCollapsibleNavBeta), and for other components (EuiFlyout and EuiOverlayMask), it allows them to correctly and dynamically set their top position and height instead of relying on a static Sass mixin to work as expected.

I strongly recommend reviewing by commit as multiple downstream components/usages were updated separately.

QA

gh pr checkout 7144 && yarn storybook
Go to http://localhost:6006/?path=/story/euiheader--multiple-fixed-headers
Dynamically add and remove headers, and confirm that each time, the page template and flyout mask correctly accounts for the number of fixed headers on the page, including when that number is 0
Go to https://eui.elastic.co/pr_7144/#/layout/header#fixed-header
Confirm that toggling the demo header appends the fixed header below the existing docs header

General checklist

Checked in Chrome, Safari, Edge, and Firefox
Added documentation
Added or updated jest ~~and cypress tests~~
A changelog entry exists and is marked appropriately

cee-chen · 2023-08-30T02:44:16Z

src/components/header/header.tsx

+  const getHeaderOffset = useCallback(
+    () => mathWithUnits(headerHeight, (x) => x * euiFixedHeadersCount),
+    [headerHeight]
+  );


I just want to quickly note - I'm aware this approach is fairly naive, and doesn't account for consumers doing something incredibly wacky, e.g.

<EuiHeader position="fixed" /> <EuiThemeProvider modify={{ base: 10 }}> {/* The underlying header will now have a height of '30px' instead of '48px' and calculations will be incorrect */} <EuiHeader position="fixed" /> </EuiThemeProvider>

I did spike out an approach for this that uses a Map to store individual header heights, but honestly, at the end of it all when I stood there looking at the final code, I couldn't justify adding the extra complexity for something that I can almost guarantee no consumer is going to use. I'd rather wait to get an actual feature request for it first.

I'll probably still open a draft PR with the working spike and then close it to preserve the code in case some absolute madman out there does want it in the future, but I don't intend on initially shipping with support for it.

I feel this is the right first approach. If consumers come back and ask for a way to remember individual header heights, you've got the solution already sketched out and captured in comment.

Almost forgot to do this! Link to the archived map code: #7152

kibanamachine · 2023-08-30T02:53:22Z

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

cee-chen · 2023-08-30T03:01:33Z

src/components/header/header.tsx

+    // Get the top position from the offset of previous header(s)
+    setTopPosition(getHeaderOffset());


Also want to note that this is not dynamic and the current Storybook QA relies on First In Last Out render order for positioning and z-index to work as expected.

Basically, if consumers do something like incredibly wacky like this:

<EuiHeader position="fixed" /> {conditionalHeader && <EuiHeader position="fixed" />} <EuiHeader position={maybeFixed? 'fixed' : 'static' /> <EuiHeader position="fixed" />

The 4th header's position will not update if the 2nd & 3rd header are dynamically removed from the fixed headers count.

Again, I find it infinitesimally unlikely that this is going to be a production use-case for someone, but if we get a feature request for it in the future, the above map spike that I mentioned will also solve dynamic addition/removal.

Link to the archived map code that correctly updates position/z-index: #7152

kibanamachine · 2023-08-30T03:20:15Z

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

cee-chen · 2023-08-30T16:33:46Z

buildkite test this

cee-chen · 2023-08-30T16:40:49Z

Hmmm, not sure what's going on with buildkite/CI... going to do a full rebase here in a min

- logic will soon become complex enough that it should be separated by concern / shouldn't simply live in the main header component

…nter

- note: this is not completely dynamic, as removed headers will not affect subsequent headers, but this is probably not super likely right now

+ update to account for EuiDataGrid's full screen mode

- cleans up a lot of now-unnecessary JS logic

- allows us to clean up a ton of now-unnecessary JS (and remove the resulting test) - add a minor workaround in style file (as opposed to inheriting EuiFlyout's CSS) - nav state rerender is noticeably more jumpy between stories

- thankfully appears to be rerender shenanigans fixed with just a quick async tick type change

- rename old counter var to more closely match new var names + update comment - DRY out repeated `mathWithUnits()` logic

kibanamachine · 2023-08-30T17:13:12Z

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

cee-chen · 2023-08-30T17:31:59Z

Alrighty, CI is passing and PR is ready for review whenever

1Copenut

Two small copy suggestions, then I'm 👍

1Copenut · 2023-08-31T15:53:53Z

src/components/header/header.tsx

+  const getHeaderOffset = useCallback(
+    () => mathWithUnits(headerHeight, (x) => x * euiFixedHeadersCount),
+    [headerHeight]
+  );


I feel this is the right first approach. If consumers come back and ask for a way to remember individual header heights, you've got the solution already sketched out and captured in comment.

src-docs/src/views/header/header_example.js

Co-authored-by: Trevor Pierce <[email protected]>

kibanamachine · 2023-08-31T17:49:56Z

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

elasticmachine · 2023-08-31T18:04:28Z

💚 Build Succeeded

Buildkite Build
Commit: 2b19e70

History

💔 Build #495 failed ca352f7
💚 Build #488 succeeded e92c72f
💔 Build #486 failed dc9d36f
💔 Build #483 failed 420dd23
💔 Build #482 failed 79fd2d0

1Copenut

👍 LGTM!

Major changes in this PR: ## Removal of `.euiAccordionForm` classNames EUI is moving away from providing global `classNames` styles for components - where possible, we want to provide props as opposed to styles. As part of our ongoing Emotion conversion, we have removed the following `EuiAccordion`-specific classes: - `.euiAccordionForm` (replaced with `borders="horizontal"`) - `.euiAccordionForm__button` (replaced with `buttonProps={{ paddingSize: 'm' }}`) - `.euiAccordionForm__title` styles - this was only removing text underlines on hover. If still desired, re-add this behavior with custom CSS. - `.euiAccordionForm__extraAction` - there was 1 usage of this in Kibana in Watcher, which was converted to one-off custom inline Emotion CSS instead. This change accounts for the first 3-4 commits in the PR. ⚠️ If your team was one of the 4-5 teams affected by this change, we would greatly appreciate your help QAing the UI and ensuring it looks as desired/the same as before. ## Fixed `EuiHeader` affordance The Sass `euiHeaderAffordForFixed` mixin has been deprecated and replaced by a global `--euiFixedHeadersOffset` CSS variable. This variable updates independently and dynamically based on the number of fixed headers on the page, and is usable by both Sass and Emotion. All underlying EUI components that need to account for fixed headers (such as flyouts and page sidebars/templates) have been updated to consume this new variable. This change cleans up a great deal of Sass code, and is incidentally extremely timely with serverless efforts (as serverless has only one fixed header and the classic Kibana chrome has two). This change constitutes a majority of the commits in this PR, which involve removing various fixed Sass header variables and replacing them with the new CSS variable. I strongly recommend [reviewing changes by commit if possible](https://github.com/elastic/kibana/pull/165790/commits) to see any associated changes that make sense together with any of your touched file(s). ⚠️ If your team was affected by this change (primarily due to custom header layouts), your help would be greatly appreciated QAing your app to ensure no UI regressions in that regard have occurred. --- `v88.1.0`⏩ `v88.2.0` ## [`88.2.0`](https://github.com/elastic/eui/tree/v88.2.0) - Added a new `EuiTextTruncate` component, which provides custom truncation options beyond native CSS ([#7116](elastic/eui#7116)) - Fixed-positioned `EuiHeader`s now set a global CSS `--euiFixedHeadersOffset` variable, which updates dynamically based on the number of fixed headers on the page. ([#7144](elastic/eui#7144)) - `EuiFlyout`s now dynamically set their position, height, and mask based on the number of fixed headers on the page. ([#7144](elastic/eui#7144)) - Sticky-positioned `EuiPageSidebar`s now dynamically set their position and height based on the number of fixed headers on the page. This can still be overridden via the `sticky.offset` prop if needed. ([#7144](elastic/eui#7144)) - `EuiPageTemplate` now dynamically offsets content from any fixed headers on the page. This can still be overridden via the `offset` prop if needed. ([#7144](elastic/eui#7144)) - Updated `EuiAccordion` with a new `borders` prop ([#7154](elastic/eui#7154)) - Updated `EuiAccordion` with a new `buttonProps.paddingSize` prop ([#7154](elastic/eui#7154)) **Deprecations** - Deprecated the Sass `euiHeaderAffordForFixed` mixin. Use the new global CSS `var(--euiFixedHeadersOffset)` variable instead. ([#7144](elastic/eui#7144)) **CSS-in-JS conversions** - Except for generic CSS utilities, EUI is moving away from providing global `classNames` that are component-specific. As part of this effort, we have removed the following `EuiAccordion`-specific classes: ([#7154](elastic/eui#7154)) - Removed `.euiAccordionForm` styles. Use the `borders="horizontal"` prop instead - Removed `.euiAccordionForm__button` styles. Use the `buttonProps={{ paddingSize: 'm' }}` prop instead - Removed `.euiAccordionForm__extraAction` styles. Convert this to your own custom CSS if necessary. - Removed `.euiAccordionForm__title` styles. Convert this to your own custom CSS if necessary. --------- Co-authored-by: Stratoula Kalafateli <[email protected]>

cee-chen requested a review from a team August 30, 2023 02:37

cee-chen marked this pull request as ready for review August 30, 2023 02:37

cee-chen requested a review from 1Copenut August 30, 2023 02:38

cee-chen commented Aug 30, 2023

View reviewed changes

cee-chen force-pushed the header-css-var branch from 79fd2d0 to 420dd23 Compare August 30, 2023 02:56

cee-chen commented Aug 30, 2023

View reviewed changes

cee-chen force-pushed the theme-css-variables branch from d2fec47 to 44d996d Compare August 30, 2023 16:41

cee-chen added 14 commits August 30, 2023 09:41

[setup] Create separate internal component for fixed headers

761e7e9

- logic will soon become complex enough that it should be separated by concern / shouldn't simply live in the main header component

Update fixed header components to set a CSS variable based on the cou…

ad9cfce

…nter

Set header top position based on number of fixed headers

265de1e

- note: this is not completely dynamic, as removed headers will not affect subsequent headers, but this is probably not super likely right now

Set up Storybook for QA

ac3b8e3

[EuiFlyout] Update to automatically adjust to fixed headers

c158539

+ update to account for EuiDataGrid's full screen mode

[EuiPageTemplate] Update to use new CSS variable

efb8d65

- cleans up a lot of now-unnecessary JS logic

[EuiCollapsibleNavBeta] Update to use new CSS variable

f680294

- allows us to clean up a ton of now-unnecessary JS (and remove the resulting test) - add a minor workaround in style file (as opposed to inheriting EuiFlyout's CSS) - nav state rerender is noticeably more jumpy between stories

[docs] Remove now-unnecessary header CSS/mixin

9690c16

[dx] Update docs, warn for deprecated Sass mixin

0d076e6

[docs] Fix broken-again side nav scroll logic

429c1c0

- thankfully appears to be rerender shenanigans fixed with just a quick async tick type change

misc cleanup

c87180d

- rename old counter var to more closely match new var names + update comment - DRY out repeated `mathWithUnits()` logic

changelog

4297fd8

Sass lint fixes

340a4ba

Update downstream flyout/overlay mask snapshots

e92c72f

cee-chen force-pushed the header-css-var branch from dc9d36f to e92c72f Compare August 30, 2023 16:42

1Copenut reviewed Aug 31, 2023

View reviewed changes

[PR feedback] Copy

ca352f7

Co-authored-by: Trevor Pierce <[email protected]>

prettier

2b19e70

cee-chen requested a review from 1Copenut August 31, 2023 20:02

1Copenut approved these changes Aug 31, 2023

View reviewed changes

cee-chen merged commit f047795 into elastic:theme-css-variables Sep 1, 2023

cee-chen deleted the header-css-var branch September 1, 2023 16:25

This was referenced Sep 1, 2023

[EuiHeader] Set up CSS variable for fixed header affordance; [Emotion] Set up CSS variable architecture #7151

Merged

[EuiHeader] Scrapped spike - complex map logic for fixed headers #7152

Closed

Upgrade EUI to v88.2.0 elastic/kibana#165790

Merged

cee-chen mentioned this pull request Oct 6, 2023

Remove deprecated euiHeader Sass mixin and variables #7264

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiHeader] Update to set a new global `--euiFixedHeadersOffset` CSS variable #7144

[EuiHeader] Update to set a new global `--euiFixedHeadersOffset` CSS variable #7144

cee-chen commented Aug 30, 2023 •

edited by 1Copenut

Loading

cee-chen Aug 30, 2023 •

edited

Loading

1Copenut Aug 31, 2023

cee-chen Sep 1, 2023

kibanamachine commented Aug 30, 2023

cee-chen Aug 30, 2023

cee-chen Sep 1, 2023

kibanamachine commented Aug 30, 2023

cee-chen commented Aug 30, 2023

cee-chen commented Aug 30, 2023

kibanamachine commented Aug 30, 2023

cee-chen commented Aug 30, 2023

1Copenut left a comment

1Copenut Aug 31, 2023

kibanamachine commented Aug 31, 2023

elasticmachine commented Aug 31, 2023

1Copenut left a comment

		// Get the top position from the offset of previous header(s)
		setTopPosition(getHeaderOffset());

[EuiHeader] Update to set a new global --euiFixedHeadersOffset CSS variable #7144

[EuiHeader] Update to set a new global --euiFixedHeadersOffset CSS variable #7144

Conversation

cee-chen commented Aug 30, 2023 • edited by 1Copenut Loading

Summary

QA

General checklist

cee-chen Aug 30, 2023 • edited Loading

Choose a reason for hiding this comment

1Copenut Aug 31, 2023

Choose a reason for hiding this comment

cee-chen Sep 1, 2023

Choose a reason for hiding this comment

kibanamachine commented Aug 30, 2023

cee-chen Aug 30, 2023

Choose a reason for hiding this comment

cee-chen Sep 1, 2023

Choose a reason for hiding this comment

kibanamachine commented Aug 30, 2023

cee-chen commented Aug 30, 2023

cee-chen commented Aug 30, 2023

kibanamachine commented Aug 30, 2023

cee-chen commented Aug 30, 2023

1Copenut left a comment

Choose a reason for hiding this comment

1Copenut Aug 31, 2023

Choose a reason for hiding this comment

kibanamachine commented Aug 31, 2023

elasticmachine commented Aug 31, 2023

💚 Build Succeeded

History

1Copenut left a comment

Choose a reason for hiding this comment

[EuiHeader] Update to set a new global `--euiFixedHeadersOffset` CSS variable #7144

[EuiHeader] Update to set a new global `--euiFixedHeadersOffset` CSS variable #7144

cee-chen commented Aug 30, 2023 •

edited by 1Copenut

Loading

cee-chen Aug 30, 2023 •

edited

Loading