Toolbar: Move ExtensionSidebar next to toolbar #113404
Conversation
svennergr
added
no-backport
| } | ||
|
|
||
| const getStyles = (theme: GrafanaTheme2) => { | ||
| const getStyles = (theme: GrafanaTheme2, extensionSidebarWidth = 0) => { |
There was a problem hiding this comment.
| const getStyles = (theme: GrafanaTheme2, extensionSidebarWidth = 0) => { | |
| const getStyles = (theme: GrafanaTheme2, extensionSidebarWidth: number) => { |
| const headerLevels = useChromeHeaderLevels(); | ||
| const headerHeight = headerLevels * getChromeHeaderLevelHeight(); | ||
| const styles = useStyles2(getStyles, headerHeight); | ||
| const sidebarTop = headerLevels === 2 ? getChromeHeaderLevelHeight() : headerHeight; |
There was a problem hiding this comment.
Do you have an example of when we have more than 2 headers? Because with 1 or 2 headers, the sidebarTop is always 40. So I want to test it when we have more than 2 headers.
There was a problem hiding this comment.
Similar to the other comment, this was not needed in the end. But to answer your question: useChromeHeaderLevels() will only return 0, 1 or 2
| [styles.content]: true, | ||
| [styles.contentChromeless]: state.chromeless, | ||
| [styles.contentWithSidebar]: isExtensionSidebarOpen && !state.chromeless, | ||
| [styles.contentWithSidebar]: isExtensionSidebarOpen && !state.chromeless && headerLevels !== 2, |
There was a problem hiding this comment.
I am not sure what does headerLevels !== 2 do here and couple lines bellow. Also what do you think about extracting headerLevels === 2 into variable that has meaningful name for its purpose and using it everywhere.
There was a problem hiding this comment.
Thanks for spotting this Ivana. Actually this introduced a regression with the position of scrollbars. I was able to just remove it from here.
2 participants
What is this feature?
Previously the ExtensionSidebar was rendering below the toolbars. Especially in Dashboards and Explore that created a bad UX where toolbar buttons where too far away from the content it belongs to:
This PR moves the ExtensionSidebar next to the toolbar, if it has two header levels:
