-
Notifications
You must be signed in to change notification settings - Fork 820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[EuiPageTemplate] Account for new serverless header style #7765
Comments
Minor correction: there are 2 fixed headers on ESS, not 1: the black header and then the white header below that. As far as If you want EuiPageTemplate to account for this new "header" type, we likely need a specific EUI component for it, likely an CC @MichaelMarcialis / @ryankeairns for design input on the above: is the second screenshot part of the official header for serverless? |
Hi EUI Team, do you have a view on when this might be looked into? We are planning where we are with the Observability solution in Kibana for serverless and this is on our radar for elastic/kibana#176177. Thanks. |
We can look into it now if y'all need - I know serverless work is high priority. What's your preferred timeline on this? |
Thanks for the ping, @cee-chen. When we were originally designing the serverless navigation, we consolidated Kibana's then double fixed headers (one dark, one light) down to a single fixed header. However, by reducing the number of fixed headers, we were left with no home for the conditionally appearing action buttons that are present in some Kibana applications (which would typically appear right aligned in the second fixed header). For that reason, we suggested that these action buttons should be housed in a conditionally appearing toolbar container. This conditionally appearing toolbar was intended to be positioned at the top of the viewport, immediately below the new singular header. It shouldn't scroll with the rest of the page content, allowing it to be visible and within reach at all times (similar to the Kibana header). Unlike the Kibana header however, it would be adjacent to the navigation (rather than above it). As such, the toolbar would need to be treated as part of the flow in regard to the navigation being collapsed or expanded (making the toolbar wider or narrower respectively). Will share some older mockups below that may do a better job conveying this. Let me know if that makes sense. CCing @ryankeairns to keep me honest. |
Should flyouts sit above it or below it? Kibana headers are above FWIW EDIT: Whoops, I see that in the 2nd to last screenshot now - it sits below flyout overlays. Yeah, this is definitely going to need a separate component from EuiHeader. |
My current work is the stateful side nav 😊 which does not touch the page templates. I would need to go into the code to see how those serverless actions are rendered in Kibana currently. I would have thought that this could be solved with CSS (some class added to the wrapping div) but I trust your judgment if a component is required 👍 |
Ah okay, @tsullivan informed me you'd taken over ownership of the sticky app bar - maybe he was mistaken? Do you know who on AppEx currently owns it? If I can't get any clear communication/ownership from either Kibana or the devs who opened this issue, I may end up closing this as a wontfix. To reiterate, the 2nd sticky bar isn't being implemented or created by EUI, it lives in Kibana, so this isn't an EUI bug or issue. We can help resolve it, but not without clearer cooperation from Kibana's side of things. |
I didn't said our team didn't own it, you asked "would implementing a new component work well with your current work ". And I answered that my current work was not related to the sticky app bar. I just looked now at the code, I don't know why we can't solve this with CSS and why we need a new component.
I see 2 solutions:
@dominiqueclarke, if we have a way to override the css of the page template and we export a CSS var for the app menu bar height, it seems that we should be able to get this issue fixed without EUI |
@sebelga @dominiqueclarke page_template_inner.tsx could also consider the
|
I can raise a PR fixing it as part of elastic/kibana#176177. @dominiqueclarke , wdyt? |
That'd be awesome. Let me know if you need any help. Cheers |
yeah, based on my tests this should fix the problem. the PR is almost ready.
|
Thanks @crespocarlos! Should I go ahead and close this issue in the EUI repo, since y'all have come up with a fix on Kibana's side of things? |
…ts and page size (#185874) fixes [176177](#176177) relates to elastic/eui#7765 ## Summary Changes the `min-block-size` to account for the `kibanaProjectHeader` element, which exists only in serverless, to calculate the page size and sticky elements' top position. <img width="1728" alt="image" src="https://github.com/elastic/kibana/assets/2767137/3b3674dc-0f3a-4671-b5e9-0b2263c2c982"> ## Stateful https://github.com/elastic/kibana/assets/2767137/2383aa9d-eee1-4aaa-94af-081f53ef84a4 ## Serverless https://github.com/elastic/kibana/assets/2767137/6830e136-cb55-4bef-8a8c-2eaf3bdc8374 https://github.com/elastic/kibana/assets/2767137/2c26014b-1572-430f-ae9c-6a4093f76422 https://github.com/elastic/kibana/assets/2767137/af6449d7-4077-4340-b5af-8a29bfae95a2 ### How to test - Start a serverless instance - `yarn es serverless --projectType=PROJECT_TYPE --kill --clean --license trial -E xpack.security.authc.api_key.enabled=true` - Start a local kibana - Check if the pages still work as expected
Hi @cee-chen, yes, you can close this out. |
I noticed that there's an unnecessary scroll on serverless when the page content does not take up the full page height.
Screen.Recording.2024-05-16.at.10.37.35.AM.mov
Although there is no additional content, the page scrolls due to the specified min-height. I noticed this style applied to the EuiPageTemplate.
This works well for ESS where there is only one fixed header.
However there are two fixed headers in serverless, requiring a different calculation.
The text was updated successfully, but these errors were encountered: