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

frontend Sidebar: Improve performance by moving isSelected logic to the parent component #2638

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
51 changes: 39 additions & 12 deletions frontend/src/components/Sidebar/Sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,8 @@ import { ActionButton } from '../common';
import CreateButton from '../common/Resource/CreateButton';
import NavigationTabs from './NavigationTabs';
import prepareRoutes from './prepareRoutes';
import SidebarItem from './SidebarItem';
import {
DefaultSidebars,
setSidebarSelected,
setWhetherSidebarOpen,
SidebarEntry,
} from './sidebarSlice';
import SidebarItem, { SidebarItemProps } from './SidebarItem';
import { DefaultSidebars, setSidebarSelected, setWhetherSidebarOpen } from './sidebarSlice';
import VersionButton from './VersionButton';

export const drawerWidth = 240;
Expand Down Expand Up @@ -153,6 +148,34 @@ const DefaultLinkArea = memo((props: { sidebarName: string; isOpen: boolean }) =
);
});

/**
* Checks if item or any sub items are selected
*/
function getIsSelected(item: SidebarItemProps, selectedName?: string | null): boolean {
if (!selectedName) return false;
return (
item.name === selectedName || Boolean(item.subList?.find(it => getIsSelected(it, selectedName)))
);
}

/**
* Updates the isSelected field of an item
*/
function updateItemSelected(
item: SidebarItemProps,
selectedName?: string | null
): SidebarItemProps {
const isSelected = getIsSelected(item, selectedName);
if (isSelected === false) return item;
return {
...item,
isSelected: isSelected,
subList: item.subList
? item.subList.map(it => updateItemSelected(it, selectedName))
: item.subList,
};
}

export default function Sidebar() {
const { t, i18n } = useTranslation(['glossary', 'translation']);

Expand All @@ -177,7 +200,7 @@ export default function Sidebar() {
return prepareRoutes(t, sidebar.selected.sidebar || '');
}, [
cluster,
sidebar.selected,
sidebar.selected.sidebar,
sidebar.entries,
sidebar.filters,
i18n.language,
Expand All @@ -195,13 +218,18 @@ export default function Sidebar() {
[sidebar.selected.sidebar, isOpen]
);

const processedItems = useMemo(
() => items.map(item => updateItemSelected(item, sidebar.selected.item)),
[items, sidebar.selected.item]
);

if (sidebar.selected.sidebar === null || !sidebar?.isVisible) {
return null;
}

return (
<PureSidebar
items={items}
items={processedItems}
open={isOpen}
openUserSelected={isUserOpened}
isNarrowOnly={isNarrowOnly}
Expand All @@ -220,7 +248,7 @@ export interface PureSidebarProps {
/** If the user has selected to open/shrink the sidebar */
openUserSelected?: boolean;
/** To show in the sidebar. */
items: SidebarEntry[];
items: SidebarItemProps[];
/** The selected route name of the sidebar open. */
selectedName: string | null;
/** If the sidebar is the temporary one (full sidebar when user selects it in mobile). */
Expand All @@ -240,7 +268,6 @@ export const PureSidebar = memo(
open,
openUserSelected,
items,
selectedName,
isTemporaryDrawer = false,
isNarrowOnly = false,
onToggleOpen,
Expand Down Expand Up @@ -292,7 +319,7 @@ export const PureSidebar = memo(
{items.map(item => (
<SidebarItem
key={item.name}
selectedName={selectedName}
isSelected={item.isSelected}
fullWidth={largeSideBarOpen}
search={search}
{...item}
Expand Down
33 changes: 4 additions & 29 deletions frontend/src/components/Sidebar/SidebarItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { SidebarEntry } from './sidebarSlice';
*/
export interface SidebarItemProps extends ListItemProps, SidebarEntry {
/** The route name which is selected. */
selectedName?: string | null;
isSelected?: boolean;
/** The navigation is a child. */
hasParent?: boolean;
/** Displayed wide with icon and text, otherwise with just a small icon. */
Expand All @@ -36,7 +36,7 @@ const SidebarItem = memo((props: SidebarItemProps) => {
search,
useClusterURL = false,
subList = [],
selectedName,
isSelected,
hasParent = false,
icon,
fullWidth = true,
Expand All @@ -61,31 +61,6 @@ const SidebarItem = memo((props: SidebarItemProps) => {
fullURL = createRouteURL(routeName);
}

const isSelected = React.useMemo(() => {
if (name === selectedName) {
return true;
}

let subListToCheck = [...subList];
for (let i = 0; i < subListToCheck.length; i++) {
const subItem = subListToCheck[i];
if (subItem.name === selectedName) {
return true;
}

if (!!subItem.subList) {
subListToCheck = subListToCheck.concat(subItem.subList);
}
}
return false;
}, [subList, name, selectedName]);

function shouldExpand() {
return isSelected || !!subList.find(item => item.name === selectedName);
}

const expanded = subList.length > 0 && shouldExpand();

return hide ? null : (
<React.Fragment>
<ListItemLink
Expand Down Expand Up @@ -221,7 +196,7 @@ const SidebarItem = memo((props: SidebarItemProps) => {
padding: 0,
}}
>
<Collapse in={fullWidth && expanded} sx={{ width: '100%' }}>
<Collapse in={fullWidth && isSelected} sx={{ width: '100%' }}>
<List
component="ul"
disablePadding
Expand All @@ -236,7 +211,7 @@ const SidebarItem = memo((props: SidebarItemProps) => {
{subList.map((item: SidebarItemProps) => (
<SidebarItem
key={item.name}
selectedName={selectedName}
isSelected={item.isSelected}
hasParent
search={search}
{...item}
Expand Down
12 changes: 6 additions & 6 deletions frontend/src/components/Sidebar/Sidebaritem.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const Template: StoryFn<SidebarItemProps> = args => {

export const Selected = Template.bind({});
Selected.args = {
selectedName: 'cluster',
isSelected: true,
name: 'cluster',
label: 'Cluster',
icon: 'mdi:hexagon-multiple-outline',
Expand All @@ -43,7 +43,7 @@ Selected.args = {

export const Unselected = Template.bind({});
Unselected.args = {
selectedName: 'meow',
isSelected: false,
name: 'cluster',
label: 'Cluster',
icon: 'mdi:hexagon-multiple-outline',
Expand All @@ -52,14 +52,14 @@ Unselected.args = {

export const SublistExpanded = Template.bind({});
SublistExpanded.args = {
selectedName: 'cluster',
isSelected: true,
name: 'cluster',
label: 'Cluster',
fullWidth: true,
icon: 'mdi:hexagon-multiple-outline',
subList: [
{
selectedName: 'cluster',
isSelected: true,
name: 'namespaces',
label: 'Namespaces',
hasParent: true,
Expand All @@ -69,14 +69,14 @@ SublistExpanded.args = {

export const Sublist = Template.bind({});
Sublist.args = {
selectedName: 'meow',
isSelected: false,
name: 'cluster',
label: 'Cluster',
fullWidth: true,
icon: 'mdi:hexagon-multiple-outline',
subList: [
{
selectedName: 'cluster',
isSelected: false,
name: 'namespaces',
label: 'Namespaces',
hasParent: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@
class="MuiList-root css-2l483y-MuiList-root"
>
<li
class="css-b4fdej"
class="css-gcxs36"
>
<a
class="MuiButtonBase-root MuiListItem-root MuiListItem-gutters MuiListItem-padding MuiListItem-button css-sayhe9-MuiButtonBase-root-MuiListItem-root"
class="MuiButtonBase-root MuiListItem-root MuiListItem-gutters MuiListItem-padding MuiListItem-button Mui-selected css-sayhe9-MuiButtonBase-root-MuiListItem-root"
href="/"
role="button"
tabindex="0"
Expand Down
Loading