[Serverless Nav] Fix issues with sticky app menu subheader #168372

Dosant · 2023-10-09T15:05:09Z

Summary

Fixes sticky kql bar in serverless security project Sticky KQL toolbar broken in Serverless #167908
Fixes double scroll in serverless discover caused by incorrect app container height cc @elastic/kibana-data-discovery

Fixes empty app header for top_nav component, for example, discover doc page:

~~Fixes empty app header for oblt onboarding page~~ Observability app is mounting an empty div into the app menu bar. #163326 Reverted because it is fixed by [Logs onboarding] Add a button 'give feedback' in the top right step by step flow #168016

packages/core/chrome/core-chrome-browser-internal/src/ui/project/app_menu.tsx

src/core/public/styles/rendering/_base.scss

…fix'

…bana into d/2023-10-09-fix-sticky-header

Dosant · 2023-10-10T14:14:37Z

packages/core/chrome/core-chrome-browser-internal/src/ui/project/app_menu.tsx

@@ -28,7 +29,7 @@ export const AppMenuBar = ({ headerActionMenuMounter }: AppMenuBarProps) => {
        display: flex;
        justify-content: end;
        align-items: center;
-        padding: ${euiTheme.size.s};
+        height: var(--kbnProjectHeaderAppActionMenuHeight, ${euiTheme.size.xxxl});


Making the app menu height fixed.

It is also important to not render the menu if there are no actions. There is no good way for app menu component to know if there is content inside. We'd try to improve places where apps set the menu mount point, so that apps stopped setting the mount point if there is no actions

This feels super elegant - it's also great that a plugin could theoretically update the --kbnProjectHeaderAppActionMenuHeight CSS variable and have everything update automatically, if they don't want to use the default xxxl height. Feels really flexible / future-proof!

Dosant · 2023-10-10T14:15:28Z

src/core/public/_mixins.scss

-  // header banner, and is visible or hidden
-  height: calc(100vh - var(--euiFixedHeadersOffset, 0) - #{$additionalOffset});
+  // header banner, app menu, and is visible or hidden
+  height: calc(100vh - var(--kbnAppHeadersOffset, 0) - #{$additionalOffset});


Fixes double scroll in serverless discover caused by incorrect app container height

Dosant · 2023-10-10T14:19:03Z

src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx

-              {renderMenu(menuClassName)}
-            </span>
-          </MountPointPortal>
+          {(badgesEl || menuEl) && (


Fixes empty app header for top_nav component, for example, discover doc page:

Dosant · 2023-10-10T14:19:27Z

x-pack/plugins/observability_onboarding/public/components/app/header_action_menu/index.tsx

-        target="_blank"
-        color="warning"
-        iconType="editorComment"
+      <HeaderMenuPortal


Fixes empty app header for oblt onboarding page #163326

x-pack/plugins/security_solution/public/app/home/template_wrapper/global_kql_header/index.tsx

…x-sticky-header

…instead + separate `kbnBody--projectActionMenuVisible` into its own selector, to improve readability and reduce CSS specificity

…iFixedHeadersOffset` if not set

packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx

Proposed minor CSS variable cleanup

…bana into d/2023-10-09-fix-sticky-header

…x-sticky-header

elasticmachine · 2023-10-11T11:54:19Z

Pinging @elastic/appex-sharedux (Team:SharedUX)

tsullivan

LGTM

Reviewed the code changes
Tested the Security Alerts page issue locally

cee-chen

🎉 🎉 Thanks for this amazingly clean fix Anton!!! I love it!

ryankeairns

Thanks Anton!

I don't want to block this obvious improvement so approving with a follow-up item.

I'll create a separate issue to track the layering where the app menu overlaps the global header, as seen below:

…x-sticky-header

Dosant · 2023-10-13T10:02:47Z

I don't want to block this obvious improvement so approving with a follow-up item.

I'll create a separate issue to track the layering where the app menu overlaps the global header, as seen below:

Thanks! I took a closer look. Seems like this is not a regression from this pr 😮‍💨, so should be good to continue with merging this

semd

Love it! thanks @Dosant
LGTM

…x-sticky-header # Conflicts: # x-pack/plugins/observability_onboarding/public/application/app.tsx

sebelga · 2023-10-13T11:13:05Z

I'll create a separate issue to track the layering where the app menu overlaps the global header, as seen below:

@ryankeairns Would have to double check but I think I fixed it in my work for the panel. Had to have it on top of anything else. So the global header (where the side nav + panel lives) is on top of everything... But not merged in main.

Can you cross link my PR (#167774) in your issue and make sure my PR closes it? Cheers!

kibana-ci · 2023-10-13T13:24:23Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 6b45ad9

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`navigation`	16	32	+16

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`kibanaReact`	138	137	-1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`discover`	574.8KB	575.0KB	+224.0B
`links`	25.2KB	25.3KB	+56.0B
`maps`	2.8MB	2.8MB	+56.0B
`securitySolution`	13.0MB	13.0MB	+27.0B
total			+363.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`core`	366.3KB	367.4KB	+1.1KB
`kibanaReact`	47.8KB	47.2KB	-568.0B
`navigation`	9.8KB	11.6KB	+1.8KB
`painlessLab`	9.9KB	9.9KB	+56.0B
`searchprofiler`	19.4KB	19.4KB	+56.0B
total			+2.5KB

Unknown metric groups

API count

id	before	after	diff
`kibanaReact`	176	173	-3

References to deprecated APIs

id	before	after	diff
`navigation`	3	0	-3

History

💔 Build #167682 failed bf2389f
💛 Build #166928 was flaky 50c30f6
💔 Build #166679 failed fa1f9d7
💔 Build #166648 failed 0a2a694
💔 Build #166361 failed e06aeb3

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @Dosant

…68372) ## Summary - Fixes sticky kql bar in serverless security project elastic#167908 - Fixes double scroll in serverless discover caused by incorrect app container height cc @elastic/kibana-data-discovery ![Screenshot 2023-10-10 at 17 23 58](https://github.com/elastic/kibana/assets/7784120/3bf50299-7d9f-4c38-953a-33a6a75815c6) - Fixes empty app header for top_nav component, for example, discover doc page: ![Screenshot 2023-10-10 at 17 24 45](https://github.com/elastic/kibana/assets/7784120/4965deac-9472-402f-8e8e-66ede83ce1bb) --------- Co-authored-by: Cee Chen <constance.chen@elastic.co>

fix sticky header attempt 1

e06aeb3

Dosant commented Oct 9, 2023

View reviewed changes

Dosant and others added 4 commits October 10, 2023 16:02

wip

bdf5f47

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

b43361d

…fix'

fix

8017506

Merge branch 'd/2023-10-09-fix-sticky-header' of github.com:Dosant/ki…

0a2a694

…bana into d/2023-10-09-fix-sticky-header

Dosant commented Oct 10, 2023

View reviewed changes

Dosant and others added 5 commits October 10, 2023 17:16

improve mount portal

d8c2ebb

improve

fa1f9d7

Merge branch 'main' of github.com:elastic/kibana into d/2023-10-09-fi…

895c3dc

…x-sticky-header

Remove --kbnAppHeadersOffset from root and only set it on the body …

4e866a7

…instead + separate `kbnBody--projectActionMenuVisible` into its own selector, to improve readability and reduce CSS specificity

Update all instances of --kbnAppHeadersOffset to fall back to `--eu…

9fc3c18

…iFixedHeadersOffset` if not set

cee-chen mentioned this pull request Oct 11, 2023

Proposed minor CSS variable cleanup Dosant/kibana#8

Merged

cee-chen reviewed Oct 11, 2023

View reviewed changes

packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx Outdated Show resolved Hide resolved

Dosant mentioned this pull request Oct 11, 2023

Observability app is mounting an empty div into the app menu bar. #163326

Closed

Dosant added 4 commits October 11, 2023 12:03

Merge pull request #8 from cee-chen/d/2023-10-09-fix-sticky-header

39d62e2

Proposed minor CSS variable cleanup

Merge branch 'd/2023-10-09-fix-sticky-header' of github.com:Dosant/ki…

fa6ed91

…bana into d/2023-10-09-fix-sticky-header

improve

0153c56

Merge branch 'main' of github.com:elastic/kibana into d/2023-10-09-fi…

50c30f6

…x-sticky-header

Dosant changed the title ~~fix sticky header attempt 1~~ [Serverless Nav] Fix issues with sticky app menu subheader Oct 11, 2023

Dosant marked this pull request as ready for review October 11, 2023 11:54

Dosant requested review from a team as code owners October 11, 2023 11:54

Dosant requested review from cee-chen, tsullivan and semd October 11, 2023 11:54

Dosant self-assigned this Oct 11, 2023

tsullivan approved these changes Oct 11, 2023

View reviewed changes

cee-chen approved these changes Oct 12, 2023

View reviewed changes

ryankeairns approved these changes Oct 12, 2023

View reviewed changes

ryankeairns mentioned this pull request Oct 12, 2023

[Serverless Nav] App menu overlaps global header when scrolling up on Mac touchpad #168713

Closed

Merge branch 'main' of github.com:elastic/kibana into d/2023-10-09-fi…

72f6748

…x-sticky-header

semd approved these changes Oct 13, 2023

View reviewed changes

Dosant added 3 commits October 13, 2023 12:23

Merge branch 'main' of github.com:elastic/kibana into d/2023-10-09-fi…

96c8d19

…x-sticky-header # Conflicts: # x-pack/plugins/observability_onboarding/public/application/app.tsx

revert oblt change

bf2389f

revert oblt change

6b45ad9

Dosant removed the request for review from a team October 13, 2023 10:27

Dosant merged commit 326ef31 into elastic:main Oct 13, 2023
35 checks passed

kibanamachine added v8.12.0 backport:skip This commit does not require backporting labels Oct 13, 2023

crespocarlos mentioned this pull request Jun 10, 2024

[EuiPageTemplate] Account for new serverless header style elastic/eui#7765

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serverless Nav] Fix issues with sticky app menu subheader #168372

[Serverless Nav] Fix issues with sticky app menu subheader #168372

Dosant commented Oct 9, 2023 •

edited

Loading

Dosant Oct 10, 2023

cee-chen Oct 11, 2023 •

edited

Loading

Dosant Oct 10, 2023 •

edited

Loading

Dosant Oct 10, 2023 •

edited

Loading

Dosant Oct 10, 2023 •

edited

Loading

elasticmachine commented Oct 11, 2023

tsullivan left a comment

cee-chen left a comment

ryankeairns left a comment

Dosant commented Oct 13, 2023

semd left a comment

sebelga commented Oct 13, 2023

kibana-ci commented Oct 13, 2023

API count

References to deprecated APIs

[Serverless Nav] Fix issues with sticky app menu subheader #168372

[Serverless Nav] Fix issues with sticky app menu subheader #168372

Conversation

Dosant commented Oct 9, 2023 • edited Loading

Summary

Dosant Oct 10, 2023

Choose a reason for hiding this comment

cee-chen Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

Dosant Oct 10, 2023 • edited Loading

Choose a reason for hiding this comment

Dosant Oct 10, 2023 • edited Loading

Choose a reason for hiding this comment

Dosant Oct 10, 2023 • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Oct 11, 2023

tsullivan left a comment

Choose a reason for hiding this comment

cee-chen left a comment

Choose a reason for hiding this comment

ryankeairns left a comment

Choose a reason for hiding this comment

Dosant commented Oct 13, 2023

semd left a comment

Choose a reason for hiding this comment

sebelga commented Oct 13, 2023

kibana-ci commented Oct 13, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

References to deprecated APIs

History

Dosant commented Oct 9, 2023 •

edited

Loading

cee-chen Oct 11, 2023 •

edited

Loading

Dosant Oct 10, 2023 •

edited

Loading

Dosant Oct 10, 2023 •

edited

Loading

Dosant Oct 10, 2023 •

edited

Loading