Skip to content

[Embedded Console] Introduce kbnSolutionNavOffset CSS variable #175348

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.

Sign up for GitHub

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

Merged
TattdCodeMonkey merged 6 commits into from
Jan 24, 2024
Merged

[Embedded Console] Introduce kbnSolutionNavOffset CSS variable #175348

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5312dbb
introduce kbnSolutionNavOffset global css variable
TattdCodeMonkey Jan 23, 2024
004ff2b
update embedded console to use correct offset
TattdCodeMonkey Jan 23, 2024
1e6fb1a
bump dataViewManagement 40B to account for SolutionNav change
TattdCodeMonkey Jan 24, 2024
2fe79bf
use euiTheme for size instead of importing euiThemeVars
TattdCodeMonkey Jan 24, 2024
412d25f
doc: add comment about JS var for nav size
TattdCodeMonkey Jan 24, 2024
5a01641
css: introduce variables for solution nav widths
TattdCodeMonkey Jan 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion packages/shared-ux/page/solution_nav/src/solution_nav.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/
import './solution_nav.scss';

import React, { FC, useState, useMemo } from 'react';
import React, { FC, useState, useMemo, useEffect } from 'react';
import classNames from 'classnames';
import {
EuiAvatarProps,
Expand All @@ -23,10 +23,12 @@ import {
htmlIdGenerator,
useIsWithinBreakpoints,
useIsWithinMinBreakpoint,
useEuiThemeCSSVariables,
} from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n-react';
import { i18n } from '@kbn/i18n';
import { KibanaSolutionAvatar } from '@kbn/shared-ux-avatar-solution';
import { euiThemeVars } from '@kbn/ui-theme';

import { SolutionNavCollapseButton } from './collapse_button';

Expand Down Expand Up @@ -174,6 +176,32 @@ export const SolutionNav: FC<SolutionNavProps> = ({
);
}, [children, headingID, isCustomSideNav, isHidden, items, rest]);

const navWidth = useMemo(() => {
if (isLargerBreakpoint) {
return isOpenOnDesktop ? `${FLYOUT_SIZE}px` : euiThemeVars.euiSizeXXL;
}
if (isMediumBreakpoint) {
return isSideNavOpenOnMobile || !canBeCollapsed
? `${FLYOUT_SIZE}px`
: euiThemeVars.euiSizeXXL;

@Dosant Dosant Jan 24, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there is no way (or no straightforward way) to connect these widths with the widths that are actually set for the sidebar. Maybe it is worth at least adding a comment in the CSS where these widths are set, that this logic might also need an update in case the CSS is updated

like here

kibana/packages/shared-ux/page/solution_nav/src/with_solution_nav.tsx

Line 77 in 51569b4

minWidth: isSidebarShrunk ? euiTheme.size.xxl : undefined,

or here

kibana/packages/shared-ux/page/solution_nav/src/solution_nav.scss

Line 17 in c39352f

width: 248px;

@Dosant Dosant Jan 24, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe it is worth at least connecting them with css variable

@TattdCodeMonkey TattdCodeMonkey Jan 24, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dosant I added variables for the sizes and added a comment about it also being set in the tsx file.

I don't think there is way to read the variables from the scss file in the tsx file at compile time without configuration changes. But if I'm wrong about that let me know.

}
return '0';
}, [
isOpenOnDesktop,
isSideNavOpenOnMobile,
canBeCollapsed,
isMediumBreakpoint,
isLargerBreakpoint,
]);
const { setGlobalCSSVariables } = useEuiThemeCSSVariables();
// Setting a global CSS variable with the nav width
// so that other pages have it available when needed.
useEffect(() => {
setGlobalCSSVariables({
'--kbnSolutionNavOffset': navWidth,
});

@Dosant Dosant Jan 24, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this should be cleaned in the destroy callback?

@TattdCodeMonkey TattdCodeMonkey Jan 24, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had that initially, but it didn't seem to change anything so I removed it as superfluous.

Also the eui code I modeled this off of does not have a destroy callback either.

@Dosant Dosant Jan 24, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it could be a problem when this variable is used with a fallback for the case when solution nav is not rendered, e.g.:

var(--kbnSolutionNavOffset, 0);

}, [navWidth, setGlobalCSSVariables]);

return (
<>
{isSmallerBreakpoint && (
Expand Down
1 change: 1 addition & 0 deletions packages/shared-ux/page/solution_nav/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@
"@kbn/i18n",
"@kbn/i18n-react",
"@kbn/shared-ux-avatar-solution",
"@kbn/ui-theme",
]
}
13 changes: 12 additions & 1 deletion src/plugins/console/public/application/containers/embeddable/_embeddable_console.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
box-shadow: inset 0 $embeddableConsoleInitialHeight 0 $embeddableConsoleBackground, inset 0 600rem 0 $euiPageBackgroundColor;
bottom: 0;
right: 0;
left: var(--euiCollapsibleNavOffset, 0);
transform: translateY(0);
height: $embeddableConsoleInitialHeight;
max-height: $embeddableConsoleMaxHeight;
Expand All @@ -18,6 +17,18 @@
z-index: $euiZLevel1;
}

&--projectChrome {
left: var(--euiCollapsibleNavOffset, 0);
}

&--classicChrome {
left: var(--kbnSolutionNavOffset, 0);
}

&--unknownChrome {
left: 0;
}

&-isOpen {
animation-duration: $euiAnimSpeedNormal;
animation-timing-function: $euiAnimSlightResistance;
Expand Down
5 changes: 5 additions & 0 deletions src/plugins/console/public/application/containers/embeddable/embeddable_console.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import React, { useState } from 'react';
import classNames from 'classnames';
import useObservable from 'react-use/lib/useObservable';
import {
EuiButton,
EuiFocusTrap,
Expand Down Expand Up @@ -38,6 +39,7 @@ export const EmbeddableConsole = ({
}: EmbeddableConsoleProps & EmbeddableConsoleDependencies) => {
const [isConsoleOpen, setIsConsoleOpen] = useState<boolean>(false);
const toggleConsole = () => setIsConsoleOpen(!isConsoleOpen);
const chromeStyle = useObservable(core.chrome.getChromeStyle$());

const onKeyDown = (event: any) => {
if (event.key === keys.ESCAPE) {
Expand All @@ -52,6 +54,9 @@ export const EmbeddableConsole = ({
'embeddableConsole--large': size === 'l',
'embeddableConsole--medium': size === 'm',
'embeddableConsole--small': size === 's',
'embeddableConsole--classicChrome': chromeStyle === 'classic',
'embeddableConsole--projectChrome': chromeStyle === 'project',
'embeddableConsole--unknownChrome': chromeStyle === undefined,
'embeddableConsole--fixed': true,
'embeddableConsole--showOnMobile': false,
});
Expand Down