frontend Sidebar: Move "isSelected" logic outside individual items

Change logic that determines if the item is selected to the parent component. This allows for better memoization of individual items. Signed-off-by: Oleksandr Dubenko <[email protected]>
headlamp-k8s · Dec 12, 2024 · 11382dd · 11382dd
1 parent d0eea72
commit 11382dd
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 49 deletions.
diff --git a/frontend/src/components/Sidebar/Sidebar.tsx b/frontend/src/components/Sidebar/Sidebar.tsx
@@ -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;
@@ -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']);
 
@@ -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,
@@ -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}
@@ -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). */
@@ -240,7 +268,6 @@ export const PureSidebar = memo(
     open,
     openUserSelected,
     items,
-    selectedName,
     isTemporaryDrawer = false,
     isNarrowOnly = false,
     onToggleOpen,
@@ -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}

diff --git a/frontend/src/components/Sidebar/SidebarItem.tsx b/frontend/src/components/Sidebar/SidebarItem.tsx
@@ -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. */
@@ -36,7 +36,7 @@ const SidebarItem = memo((props: SidebarItemProps) => {
     search,
     useClusterURL = false,
     subList = [],
-    selectedName,
+    isSelected,
     hasParent = false,
     icon,
     fullWidth = true,
@@ -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
@@ -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
@@ -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}

diff --git a/frontend/src/components/Sidebar/Sidebaritem.stories.tsx b/frontend/src/components/Sidebar/Sidebaritem.stories.tsx
@@ -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',
@@ -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',
@@ -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,
@@ -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,

diff --git a/frontend/src/components/Sidebar/__snapshots__/Sidebaritem.SublistExpanded.stories.storyshot b/frontend/src/components/Sidebar/__snapshots__/Sidebaritem.SublistExpanded.stories.storyshot
@@ -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"