Skip to content
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

[Stateful sidenav] Fix highlight colours #201823

Merged
merged 11 commits into from
Nov 28, 2024
4 changes: 3 additions & 1 deletion packages/kbn-test/src/jest/setup/react_testing_library.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ jest.mock('@testing-library/react', () => {
const originalConsoleError = console.error;
console.error = (...args) => {
if (global.IS_REACT_ACT_ENVIRONMENT === false) {
if (args[0].includes('Warning: An update to %s inside a test was not wrapped in act')) {
if (
args[0].toString().includes('Warning: An update to %s inside a test was not wrapped in act')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to convert to String as the error that was passed was an "object" type that does not have the includes prop

) {
return;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ describe('Panel', () => {
id: 'group2',
title: 'Group 2',
path: 'root.group2',
href: '/app/group2',
renderAs: 'panelOpener',
children: [
// sideNavStatus is "visible" by default
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
useEuiTheme,
transparentize,
useIsWithinMinBreakpoint,
EuiButton,
} from '@elastic/eui';
import type { ChromeProjectNavigationNode } from '@kbn/core-chrome-browser';
import { useNavigation as useServices } from '../../services';
Expand All @@ -45,6 +46,24 @@ const getStyles = (euiTheme: EuiThemeComputed<{}>) => css`
}
`;

const getButtonStyles = (euiTheme: EuiThemeComputed<{}>, isActive: boolean) => css`
background-color: ${isActive ? transparentize(euiTheme.colors.lightShade, 0.5) : 'transparent'};
transform: none !important; /* don't translateY 1px */
color: inherit;
font-weight: inherit;
padding-inline: ${euiTheme.size.s};
& > span {
justify-content: flex-start;
position: relative;
}
& .euiIcon {
position: absolute;
right: 0;
top: 0;
transform: translateY(50%);
}
`;

interface Props {
item: ChromeProjectNavigationNode;
navigateToUrl: NavigateToUrlFn;
Expand All @@ -60,15 +79,18 @@ export const NavigationItemOpenPanel: FC<Props> = ({ item, navigateToUrl, active
const href = deepLink?.url ?? item.href;
const isNotMobile = useIsWithinMinBreakpoint('s');
const isIconVisible = isNotMobile && !isSideNavCollapsed && !!children && children.length > 0;
const isActive = isActiveFromUrl(item.path, activeNodes);
const hasLandingPage = Boolean(href);
const isExpanded = selectedNode?.path === path;
const isActive = hasLandingPage ? isActiveFromUrl(item.path, activeNodes) : isExpanded;

const itemClassNames = classNames(
'sideNavItem',
{ 'sideNavItem--isActive': isActive },
getStyles(euiTheme)
);

const buttonClassNames = classNames('sideNavItem', getButtonStyles(euiTheme, isActive));

const dataTestSubj = classNames(`nav-item`, `nav-item-${path}`, {
[`nav-item-deepLinkId-${deepLink?.id}`]: !!deepLink,
[`nav-item-id-${id}`]: id,
Expand Down Expand Up @@ -105,7 +127,21 @@ export const NavigationItemOpenPanel: FC<Props> = ({ item, navigateToUrl, active
togglePanel();
}, [togglePanel]);

const isExpanded = selectedNode?.path === path;
if (!hasLandingPage) {
return (
<EuiButton
onClick={onLinkClick}
iconSide="right"
iconType="arrowRight"
size="s"
fullWidth
className={buttonClassNames}
data-test-subj={dataTestSubj}
>
{title}
</EuiButton>
);
}

return (
<EuiFlexGroup alignItems="center" gutterSize="xs">
Expand All @@ -130,7 +166,7 @@ export const NavigationItemOpenPanel: FC<Props> = ({ item, navigateToUrl, active
size="s"
color="text"
onClick={onIconClick}
iconType={hasLandingPage ? 'spaces' : 'arrowRight'}
iconType="spaces"
iconSize="m"
aria-label={i18n.translate('sharedUXPackages.chrome.sideNavigation.togglePanel', {
defaultMessage: 'Toggle "{title}" panel navigation',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@

import React, { FC, useCallback } from 'react';
import type { ChromeProjectNavigationNode } from '@kbn/core-chrome-browser';
import { EuiListGroupItem } from '@elastic/eui';
import { EuiListGroupItem, transparentize, useEuiTheme } from '@elastic/eui';
import classNames from 'classnames';
import { css } from '@emotion/css';

import { useNavigation as useServices } from '../../../services';
import { NavItemLabel } from './panel_nav_item_label';
Expand All @@ -24,6 +26,7 @@ export const PanelNavItem: FC<Props> = ({ item }) => {
const { close: closePanel } = usePanel();
const { id, icon, deepLink, openInNewTab } = item;
const href = deepLink?.url ?? item.href;
const { euiTheme } = useEuiTheme();

const onClick = useCallback<React.MouseEventHandler>(
(e) => {
Expand All @@ -41,7 +44,14 @@ export const PanelNavItem: FC<Props> = ({ item }) => {
key={id}
label={<NavItemLabel item={item} />}
wrapText
className="sideNavPanelLink"
className={classNames(
'sideNavPanelLink',
css`
&.sideNavPanelLink:hover {
background-color: ${transparentize(euiTheme.colors.lightShade, 0.5)};
}
`
)}
size="s"
data-test-subj={`panelNavItem panelNavItem-id-${item.id}`}
href={href}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,12 @@ export const useAccordionState = ({ navNode }: { navNode: ChromeProjectNavigatio
const getAccordionProps = useCallback(
(
path: string,
_accordionProps?: Partial<EuiAccordionProps>
accordionProps?: Partial<EuiAccordionProps>
): Partial<EuiAccordionProps> | undefined => {
const isCollapsed = accordionStateById[path]?.isCollapsed;
const isCollapsible = accordionStateById[path]?.isCollapsible;

if (isCollapsed === undefined) return _accordionProps; // No state set yet
if (isCollapsed === undefined) return accordionProps; // No state set yet

let forceState: EuiAccordionProps['forceState'] = isCollapsed ? 'closed' : 'open';
if (!isCollapsible) forceState = 'open'; // Allways open if the accordion is not collapsible
Expand All @@ -109,9 +109,8 @@ export const useAccordionState = ({ navNode }: { navNode: ChromeProjectNavigatio

const updated: Partial<EuiAccordionProps & { isCollapsible?: boolean }> = {
buttonProps: { 'data-test-subj': 'accordionToggleBtn' },
..._accordionProps,
...accordionProps,
arrowProps,
isCollapsible,
forceState,
onToggle: isCollapsible
? () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,19 @@ export const SolutionSideNavPanelStyles = (
// bottom inset to match timeline bar top shadow
inset 0 -6px ${euiTheme.size.xs} -${euiTheme.size.xs} rgb(0 0 0 / 15%);
`}
`;

.solutionSideNavPanelLink {
&:focus-within {
background-color: transparent;
a {
text-decoration: auto;
}
export const SolutionSideNavPanelItemStyles = (euiTheme: EuiThemeComputed<{}>) => css`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to move those style outside as for the platform side nav the links are rendered in a different panel

&:focus-within {
background-color: transparent;
a {
text-decoration: auto;
}
&:hover {
background-color: ${transparentize(euiTheme.colors.primary, 0.1)};
a {
text-decoration: underline;
}
}
&:hover {
background-color: ${transparentize(euiTheme.colors.lightShade, 0.5)};
a {
text-decoration: underline;
}
}
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import {
SolutionSideNavPanelLinksGroupStyles,
panelClassName,
accordionButtonClassName,
SolutionSideNavPanelItemStyles,
} from './solution_side_nav_panel.styles';

export interface SolutionSideNavPanelContentProps {
Expand Down Expand Up @@ -354,7 +355,11 @@ interface SolutionSideNavPanelItemProps {
const SolutionSideNavPanelItem: React.FC<SolutionSideNavPanelItemProps> = React.memo(
function SolutionSideNavPanelItem({ item, onClose }) {
const { tracker } = useTelemetryContext();
const panelLinkClassNames = classNames('solutionSideNavPanelLink');
const { euiTheme } = useEuiTheme();
const panelLinkClassNames = classNames(
'solutionSideNavPanelLink',
sebelga marked this conversation as resolved.
Show resolved Hide resolved
SolutionSideNavPanelItemStyles(euiTheme)
);
const { id, href, onClick, iconType, openInNewTab } = item;
const onClickHandler = useCallback<React.MouseEventHandler>(
(ev) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
expect(isOpen).to.be(false);
}

// open Infrastructure panel using the icon button and navigate to some link inside the panel
await solutionNavigation.sidenav.openPanel('metrics', { button: 'icon' });
// open Infrastructure panel and navigate to some link inside the panel
await solutionNavigation.sidenav.openPanel('metrics', { button: 'link' });
{
const isOpen = await solutionNavigation.sidenav.isPanelOpen('metrics');
expect(isOpen).to.be(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export function MachineLearningNavigationProviderObservability({
const svlCommonNavigation = getPageObject('svlCommonNavigation');

async function navigateToArea(id: string) {
await svlCommonNavigation.sidenav.openPanel('machine_learning-landing');
await svlCommonNavigation.sidenav.openPanel('machine_learning-landing', { button: 'link' });
await testSubjects.existOrFail(`~panelNavItem-id-ml:${id}`, {
timeout: 60 * 1000,
});
Expand Down