Role picker: Refactor menu components (#60584)

* Simplify underlying components

* Move state management deeper to RoleMenuGroupsSection component

* Get rid of some unnecessary props passing

* Reduce number of unnecessary re-renders

* Simplify state

public/app/core/components/RolePicker/RoleMenuGroupOption.tsx

@@ -1,18 +1,20 @@
 import { cx } from '@emotion/css';
-import React, { FormEvent } from 'react';
+import React, { FormEvent, memo } from 'react';
 
-import { SelectableValue } from '@grafana/data';
 import { Checkbox, Portal, useStyles2, useTheme2 } from '@grafana/ui';
 import { getSelectStyles } from '@grafana/ui/src/components/Select/getSelectStyles';
 
 import { getStyles } from './styles';
 
 interface RoleMenuGroupsOptionProps {
-  data: SelectableValue<string>;
+  // display name
+  name: string;
+  // group id
+  value: string;
   onChange: (value: string) => void;
   onClick?: (value: string) => void;
   onOpenSubMenu?: (value: string) => void;
-  onCloseSubMenu?: (value: string) => void;
+  onCloseSubMenu?: () => void;
   isSelected?: boolean;
   partiallySelected?: boolean;
   isFocused?: boolean;
@@ -21,84 +23,87 @@ interface RoleMenuGroupsOptionProps {
   root?: HTMLElement;
 }
 
-export const RoleMenuGroupOption = React.forwardRef<HTMLDivElement, RoleMenuGroupsOptionProps>(
-  (
-    {
-      data,
-      isFocused,
-      isSelected,
-      partiallySelected,
-      disabled,
-      onChange,
-      onClick,
-      onOpenSubMenu,
-      onCloseSubMenu,
-      children,
-      root,
-    },
-    ref
-  ) => {
-    const theme = useTheme2();
-    const styles = getSelectStyles(theme);
-    const customStyles = useStyles2(getStyles);
+export const RoleMenuGroupOption = memo(
+  React.forwardRef<HTMLDivElement, RoleMenuGroupsOptionProps>(
+    (
+      {
+        name,
+        value,
+        isFocused,
+        isSelected,
+        partiallySelected,
+        disabled,
+        onChange,
+        onClick,
+        onOpenSubMenu,
+        onCloseSubMenu,
+        children,
+        root,
+      },
+      ref
+    ) => {
+      const theme = useTheme2();
+      const styles = getSelectStyles(theme);
+      const customStyles = useStyles2(getStyles);
 
-    const wrapperClassName = cx(
-      styles.option,
-      isFocused && styles.optionFocused,
-      disabled && customStyles.menuOptionDisabled
-    );
+      const wrapperClassName = cx(
+        styles.option,
+        isFocused && styles.optionFocused,
+        disabled && customStyles.menuOptionDisabled
+      );
 
-    const onChangeInternal = (event: FormEvent<HTMLElement>) => {
-      if (disabled) {
-        return;
-      }
-      if (data.value) {
-        onChange(data.value);
-      }
-    };
+      const onChangeInternal = (event: FormEvent<HTMLElement>) => {
+        if (disabled) {
+          return;
+        }
+        if (value) {
+          onChange(value);
+        }
+      };
 
-    const onClickInternal = (event: FormEvent<HTMLElement>) => {
-      if (onClick) {
-        onClick(data.value!);
-      }
-    };
+      const onClickInternal = (event: FormEvent<HTMLElement>) => {
+        if (onClick) {
+          onClick(value!);
+        }
+      };
 
-    const onMouseEnter = () => {
-      if (onOpenSubMenu) {
-        onOpenSubMenu(data.value!);
-      }
-    };
+      const onMouseEnter = () => {
+        if (onOpenSubMenu) {
+          onOpenSubMenu(value!);
+        }
+      };
 
-    const onMouseLeave = () => {
-      if (onCloseSubMenu) {
-        onCloseSubMenu(data.value!);
-      }
-    };
+      const onMouseLeave = () => {
+        if (onCloseSubMenu) {
+          onCloseSubMenu();
+        }
+      };
 
-    return (
-      <div onMouseEnter={onMouseEnter} onMouseLeave={onMouseLeave}>
-        <div ref={ref} className={wrapperClassName} aria-label="Role picker option" onClick={onClickInternal}>
-          <Checkbox
-            value={isSelected}
-            className={cx(customStyles.menuOptionCheckbox, {
-              [customStyles.checkboxPartiallyChecked]: partiallySelected,
-            })}
-            onChange={onChangeInternal}
-            disabled={disabled}
-          />
-          <div className={cx(styles.optionBody, customStyles.menuOptionBody)}>
-            <span>{data.displayName || data.name}</span>
-            <span className={customStyles.menuOptionExpand} />
+      return (
+        <div onMouseEnter={onMouseEnter} onMouseLeave={onMouseLeave}>
+          <div ref={ref} className={wrapperClassName} aria-label="Role picker option" onClick={onClickInternal}>
+            <Checkbox
+              value={isSelected}
+              className={cx(customStyles.menuOptionCheckbox, {
+                [customStyles.checkboxPartiallyChecked]: partiallySelected,
+              })}
+              onChange={onChangeInternal}
+              disabled={disabled}
+            />
+            <div className={cx(styles.optionBody, customStyles.menuOptionBody)}>
+              <span>{name}</span>
+              <span className={customStyles.menuOptionExpand} />
+            </div>
+            {root && children && (
+              <Portal className={customStyles.subMenuPortal} root={root}>
+                {children}
+              </Portal>
+            )}
           </div>
-          {root && children && (
-            <Portal className={customStyles.subMenuPortal} root={root}>
-              {children}
-            </Portal>
-          )}
         </div>
-      </div>
-    );
-  }
+      );
+    }
+  )
 );
 
 RoleMenuGroupOption.displayName = 'RoleMenuGroupOption';

public/app/core/components/RolePicker/RoleMenuGroupsSection.tsx

@@ -1,37 +1,31 @@
-import React from 'react';
+import React, { useCallback, useState } from 'react';
 
+import { useStyles2, getSelectStyles, useTheme2 } from '@grafana/ui';
 import { Role } from 'app/types';
 
 import { RoleMenuGroupOption } from './RoleMenuGroupOption';
 import { RoleMenuOption } from './RoleMenuOption';
 import { RolePickerSubMenu } from './RolePickerSubMenu';
+import { getStyles } from './styles';
 import { isNotDelegatable } from './utils';
 
 interface RoleMenuGroupsSectionProps {
   roles: Role[];
   renderedName: string;
-  menuSectionStyle: string;
-  groupHeaderStyle: string;
-  optionBodyStyle: string;
   showGroups?: boolean;
   optionGroups: Array<{
     name: string;
     options: Role[];
     value: string;
   }>;
-  onChange: (value: string) => void;
-  onOpenSubMenuRMGS: (value: string) => void;
-  onCloseSubMenu?: (value: string) => void;
+  onGroupChange: (value: string) => void;
   groupSelected: (group: string) => boolean;
   groupPartiallySelected: (group: string) => boolean;
   disabled?: boolean;
   subMenuNode?: HTMLDivElement;
-  showSubMenu: boolean;
-  openedMenuGroup: string;
-  subMenuOptions: Role[];
   selectedOptions: Role[];
-  onChangeSubMenu: (option: Role) => void;
-  onClearSubMenu: () => void;
+  onRoleChange: (option: Role) => void;
+  onClearSubMenu: (group: string) => void;
   showOnLeftSubMenu: boolean;
 }
 
@@ -40,53 +34,63 @@ export const RoleMenuGroupsSection = React.forwardRef<HTMLDivElement, RoleMenuGr
     {
       roles,
       renderedName,
-      menuSectionStyle,
-      groupHeaderStyle,
-      optionBodyStyle,
       showGroups,
       optionGroups,
-      onChange,
+      onGroupChange,
       groupSelected,
       groupPartiallySelected,
-      onOpenSubMenuRMGS,
-      onCloseSubMenu,
       subMenuNode,
-      showSubMenu,
-      openedMenuGroup,
-      subMenuOptions,
       selectedOptions,
-      onChangeSubMenu,
+      onRoleChange,
       onClearSubMenu,
       showOnLeftSubMenu,
     },
     _ref
   ) => {
+    const [showSubMenu, setShowSubMenu] = useState(false);
+    const [openedMenuGroup, setOpenedMenuGroup] = useState('');
+
+    const theme = useTheme2();
+    const selectStyles = getSelectStyles(theme);
+    const styles = useStyles2(getStyles);
+
+    const onOpenSubMenu = useCallback((value: string) => {
+      setOpenedMenuGroup(value);
+      setShowSubMenu(true);
+    }, []);
+
+    const onCloseSubMenu = useCallback(() => {
+      setShowSubMenu(false);
+      setOpenedMenuGroup('');
+    }, []);
+
     return (
       <div>
         {roles.length > 0 && (
-          <div className={menuSectionStyle}>
-            <div className={groupHeaderStyle}>{renderedName}</div>
-            <div className={optionBodyStyle}></div>
+          <div className={styles.menuSection}>
+            <div className={styles.groupHeader}>{renderedName}</div>
+            <div className={selectStyles.optionBody}></div>
             {showGroups && !!optionGroups?.length
               ? optionGroups.map((groupOption) => (
                   <RoleMenuGroupOption
-                    data={groupOption}
                     key={groupOption.value}
+                    name={groupOption.name}
+                    value={groupOption.value}
                     isSelected={groupSelected(groupOption.value) || groupPartiallySelected(groupOption.value)}
                     partiallySelected={groupPartiallySelected(groupOption.value)}
                     disabled={groupOption.options?.every(isNotDelegatable)}
-                    onChange={onChange}
-                    onOpenSubMenu={onOpenSubMenuRMGS}
+                    onChange={onGroupChange}
+                    onOpenSubMenu={onOpenSubMenu}
                     onCloseSubMenu={onCloseSubMenu}
                     root={subMenuNode}
                     isFocused={showSubMenu && openedMenuGroup === groupOption.value}
                   >
                     {showSubMenu && openedMenuGroup === groupOption.value && (
                       <RolePickerSubMenu
-                        options={subMenuOptions}
+                        options={groupOption.options}
                         selectedOptions={selectedOptions}
-                        onSelect={onChangeSubMenu}
-                        onClear={onClearSubMenu}
+                        onSelect={onRoleChange}
+                        onClear={() => onClearSubMenu(openedMenuGroup)}
                         showOnLeft={showOnLeftSubMenu}
                       />
                     )}
@@ -98,7 +102,7 @@ export const RoleMenuGroupsSection = React.forwardRef<HTMLDivElement, RoleMenuGr
                     key={option.uid}
                     isSelected={!!(option.uid && !!selectedOptions.find((opt) => opt.uid === option.uid))}
                     disabled={isNotDelegatable(option)}
-                    onChange={onChangeSubMenu}
+                    onChange={onRoleChange}
                     hideDescription
                   />
                 ))}

public/app/core/components/RolePicker/RolePickerMenu.tsx

@@ -71,10 +71,6 @@ export const RolePickerMenu = ({
 }: RolePickerMenuProps): JSX.Element => {
   const [selectedOptions, setSelectedOptions] = useState<Role[]>(appliedRoles);
   const [selectedBuiltInRole, setSelectedBuiltInRole] = useState<OrgRole | undefined>(basicRole);
-  const [showSubMenu, setShowSubMenu] = useState(false);
-  const [openedMenuGroup, setOpenedMenuGroup] = useState('');
-  const [subMenuOptions, setSubMenuOptions] = useState<Role[]>([]);
-  const [optionGroups, setOptionGroups] = useState<{ [key: string]: RoleGroupOption[] }>({});
   const [rolesCollection, setRolesCollection] = useState<{ [key: string]: RolesCollectionEntry }>({});
   const subMenuNode = useRef<HTMLDivElement | null>(null);
   const theme = useTheme2();
@@ -92,7 +88,7 @@ export const RolePickerMenu = ({
     }
   }, [selectedBuiltInRole, onBasicRoleSelect]);
 
-  // Evaluate optionGroups and rolesCollection only if options changed, otherwise
+  // Evaluate rolesCollection only if options changed, otherwise
   // it triggers unnecessary re-rendering of <RoleMenuGroupsSection /> component
   useEffect(() => {
     const customRoles = options.filter(filterCustomRoles).sort(sortRolesByName);
@@ -103,7 +99,6 @@ export const RolePickerMenu = ({
       custom: convertRolesToGroupOptions(customRoles).sort((a, b) => a.name.localeCompare(b.name)),
       plugin: convertRolesToGroupOptions(pluginRoles).sort((a, b) => a.name.localeCompare(b.name)),
     };
-    setOptionGroups(optionGroups);
 
     setRolesCollection({
       fixed: {
@@ -118,7 +113,7 @@ export const RolePickerMenu = ({
         renderedName: `Custom roles`,
         roles: customRoles,
       },
-      pluginRoles: {
+      plugin: {
         groupType: GroupType.plugin,
         optionGroup: optionGroups.plugin,
         renderedName: `Plugin roles`,
@@ -139,13 +134,13 @@ export const RolePickerMenu = ({
 
   const groupSelected = (groupType: GroupType, group: string) => {
     const selectedGroupOptions = getSelectedGroupOptions(group);
-    const groupOptions = optionGroups[groupType].find((g) => g.value === group);
+    const groupOptions = rolesCollection[groupType]?.optionGroup.find((g) => g.value === group);
     return selectedGroupOptions.length > 0 && selectedGroupOptions.length >= groupOptions!.options.length;
   };
 
   const groupPartiallySelected = (groupType: GroupType, group: string) => {
     const selectedGroupOptions = getSelectedGroupOptions(group);
-    const groupOptions = optionGroups[groupType].find((g) => g.value === group);
+    const groupOptions = rolesCollection[groupType]?.optionGroup.find((g) => g.value === group);
     return selectedGroupOptions.length > 0 && selectedGroupOptions.length < groupOptions!.options.length;
   };
 
@@ -158,7 +153,7 @@ export const RolePickerMenu = ({
   };
 
   const onGroupChange = (groupType: GroupType, value: string) => {
-    const group = optionGroups[groupType].find((g) => {
+    const group = rolesCollection[groupType]?.optionGroup.find((g) => {
       return g.value === value;
     });
 
@@ -175,23 +170,6 @@ export const RolePickerMenu = ({
     }
   };
 
-  const onOpenSubMenu = (groupType: GroupType, value: string) => {
-    setOpenedMenuGroup(value);
-    setShowSubMenu(true);
-    const group = optionGroups[groupType].find((g) => {
-      return g.value === value;
-    });
-    if (group) {
-      setSubMenuOptions(group.options);
-    }
-  };
-
-  const onCloseSubMenu = (value: string) => {
-    setShowSubMenu(false);
-    setOpenedMenuGroup('');
-    setSubMenuOptions([]);
-  };
-
   const onSelectedBuiltinRoleChange = (newRole: OrgRole) => {
     setSelectedBuiltInRole(newRole);
   };
@@ -200,10 +178,10 @@ export const RolePickerMenu = ({
     setSelectedOptions([]);
   };
 
-  const onClearSubMenu = () => {
+  const onClearSubMenu = (group: string) => {
     const options = selectedOptions.filter((role) => {
-      const groupName = getRoleGroup(role);
-      return groupName !== openedMenuGroup;
+      const roleGroup = getRoleGroup(role);
+      return roleGroup !== group;
     });
     setSelectedOptions(options);
   };
@@ -239,33 +217,23 @@ export const RolePickerMenu = ({
               />
             </div>
           )}
-          {Object.entries(rolesCollection).map(([groupId, collection]) => {
-            return (
-              <RoleMenuGroupsSection
-                key={groupId}
-                roles={collection.roles}
-                renderedName={collection.renderedName}
-                menuSectionStyle={customStyles.menuSection}
-                groupHeaderStyle={customStyles.groupHeader}
-                optionBodyStyle={styles.optionBody}
-                showGroups={showGroups}
-                optionGroups={collection.optionGroup}
-                groupSelected={(group: string) => groupSelected(collection.groupType, group)}
-                groupPartiallySelected={(group: string) => groupPartiallySelected(collection.groupType, group)}
-                onChange={(group: string) => onGroupChange(collection.groupType, group)}
-                onOpenSubMenuRMGS={(group: string) => onOpenSubMenu(collection.groupType, group)}
-                onCloseSubMenu={onCloseSubMenu}
-                subMenuNode={subMenuNode?.current!}
-                showSubMenu={showSubMenu}
-                openedMenuGroup={openedMenuGroup}
-                subMenuOptions={subMenuOptions}
-                selectedOptions={selectedOptions}
-                onChangeSubMenu={onChange}
-                onClearSubMenu={onClearSubMenu}
-                showOnLeftSubMenu={offset.horizontal > 0}
-              ></RoleMenuGroupsSection>
-            );
-          })}
+          {Object.entries(rolesCollection).map(([groupId, collection]) => (
+            <RoleMenuGroupsSection
+              key={groupId}
+              roles={collection.roles}
+              renderedName={collection.renderedName}
+              showGroups={showGroups}
+              optionGroups={collection.optionGroup}
+              groupSelected={(group: string) => groupSelected(collection.groupType, group)}
+              groupPartiallySelected={(group: string) => groupPartiallySelected(collection.groupType, group)}
+              onGroupChange={(group: string) => onGroupChange(collection.groupType, group)}
+              subMenuNode={subMenuNode?.current!}
+              selectedOptions={selectedOptions}
+              onRoleChange={onChange}
+              onClearSubMenu={onClearSubMenu}
+              showOnLeftSubMenu={offset.horizontal > 0}
+            />
+          ))}
         </CustomScrollbar>
         <div className={customStyles.menuButtonRow}>
           <HorizontalGroup justify="flex-end">