fix(VariantManagement): fix boolean prop handling (#6641)

Lukas742 · web-flow · commit 69ed3e0cbb24 · 2024-12-02T08:49:17.000+01:00
Fixes #6616
diff --git a/packages/main/src/components/VariantManagement/VariantItem.tsx b/packages/main/src/components/VariantManagement/VariantItem.tsx
@@ -4,14 +4,16 @@ import { useStylesheet, useSyncRef } from '@ui5/webcomponents-react-base';
 import { clsx } from 'clsx';
 import { forwardRef, useContext, useEffect } from 'react';
 import { VariantManagementContext } from '../../internal/VariantManagementContext.js';
-import type { InputPropTypes } from '../../webComponents/index.js';
+import type { InputPropTypes } from '../../webComponents/Input/index.js';
 import type { ListItemStandardDomRef, ListItemStandardPropTypes } from '../../webComponents/ListItemStandard/index.js';
 import { ListItemStandard } from '../../webComponents/ListItemStandard/index.js';
 import { classNames, styleData } from './VariantItem.module.css.js';
 
 export interface VariantItemPropTypes extends Pick<ListItemStandardPropTypes, 'accessibleName' | 'selected'> {
   /**
    * The name of the variant.
+   *
+   * __Note:__ Variant names must be unique.
    */
   children: string;
   /**
@@ -30,6 +32,8 @@ export interface VariantItemPropTypes extends Pick<ListItemStandardPropTypes, 'a
   global?: boolean;
   /**
    * Indicator if it's the default variant.
+   *
+   * __Note:__ There should only be __one__ default variant.
    */
   isDefault?: boolean;
   /**
@@ -90,7 +94,7 @@ const VariantItem = forwardRef<ListItemStandardDomRef, VariantItemPropTypes>((pr
 
   useStylesheet(styleData, VariantItem.displayName);
 
-  const { selectVariantItem } = useContext(VariantManagementContext);
+  const { selectVariantItem, selectedVariant } = useContext(VariantManagementContext);
   const [componentRef, consolidatedRef] = useSyncRef<ListItemStandardDomRef>(ref);
   useEffect(() => {
     if (selected) {
@@ -116,6 +120,7 @@ const VariantItem = forwardRef<ListItemStandardDomRef, VariantItemPropTypes>((pr
       data-read-only={readOnly}
       data-children={children}
       data-hide-delete={hideDelete}
+      selected={selectedVariant?.children === children}
     />
   );
 });
diff --git a/packages/main/src/components/VariantManagement/VariantManagement.cy.tsx b/packages/main/src/components/VariantManagement/VariantManagement.cy.tsx
@@ -1,6 +1,7 @@
 import TitleLevel from '@ui5/webcomponents/dist/types/TitleLevel.js';
 import ValueState from '@ui5/webcomponents-base/dist/types/ValueState.js';
 import { useState } from 'react';
+import type { VariantItemPropTypes } from './VariantItem';
 import { VariantItem } from './VariantItem';
 import { WithCustomValidation as WithCustomValidationStory } from './VariantManagement.stories';
 import type { VariantManagementPropTypes } from './index.js';
@@ -434,7 +435,7 @@ describe('VariantManagement', () => {
   });
 
   it('Manage Views - render variants', () => {
-    const variantItems = [
+    const variantItems: VariantItemPropTypes[] = [
       { rowId: 'Default VariantItem', props: {} },
       { rowId: 'LabelReadOnly', props: { labelReadOnly: true } },
       { rowId: 'Favorite', props: { favorite: true } },
@@ -689,5 +690,62 @@ describe('VariantManagement', () => {
     cy.get('@saveView').should('have.been.calledOnce');
   });
 
+  it('Programatically change selection', () => {
+    const TestComp = () => {
+      const [selected, setSelected] = useState('Item1');
+      return (
+        <>
+          <VariantManagement
+            onSelect={(e) => {
+              setSelected(e.detail.selectedVariant.children);
+            }}
+          >
+            <VariantItem selected={selected === 'Item1'}>Item1</VariantItem>
+            <VariantItem selected={selected === 'Item2'}>Item2</VariantItem>
+            <VariantItem selected={selected === 'Item3'}>Item3</VariantItem>
+            <VariantItem selected={selected === 'Item4'}>Item4</VariantItem>
+            <VariantItem selected={selected === 'Item5'}>Item5</VariantItem>
+          </VariantManagement>
+          <button
+            onClick={() => {
+              setSelected('Item3');
+            }}
+          >
+            Select Item3
+          </button>
+          <button
+            onClick={() => {
+              setSelected('Item5');
+            }}
+          >
+            Select Item5
+          </button>
+          <button
+            onClick={() => {
+              setSelected('Item1');
+            }}
+          >
+            Select Item1
+          </button>
+        </>
+      );
+    };
+    cy.mount(<TestComp />);
+
+    cy.get('[ui5-title]').contains('Item1').should('be.visible');
+    cy.findByText('Select Item3').click();
+    cy.get('[ui5-title]').contains('Item3').should('be.visible').click();
+    cy.get('[ui5-responsive-popover]').should('be.visible');
+    cy.get('[ui5-list]').contains('Item3').should('have.attr', 'selected', 'selected');
+    cy.findByText('Item1').click();
+    cy.get('[ui5-list]').contains('Item3').should('not.have.attr', 'selected');
+    cy.get('[ui5-list]').contains('Item1').should('have.attr', 'selected', 'selected');
+    cy.realPress('Escape');
+    cy.findByText('Select Item5').click();
+    cy.get('[ui5-title]').contains('Item5').should('be.visible').click();
+    cy.get('[ui5-list]').contains('Item1').should('not.have.attr', 'selected');
+    cy.get('[ui5-list]').contains('Item5').should('have.attr', 'selected', 'selected');
+  });
+
   cypressPassThroughTestsFactory(VariantManagement);
 });
diff --git a/packages/main/src/components/VariantManagement/index.tsx b/packages/main/src/components/VariantManagement/index.tsx
@@ -12,35 +12,57 @@ import searchIcon from '@ui5/webcomponents-icons/dist/search.js';
 import { enrichEventWithDetails, useI18nBundle, useStylesheet } from '@ui5/webcomponents-react-base';
 import { clsx } from 'clsx';
 import type { ComponentElement, ReactElement } from 'react';
-import { Children, cloneElement, forwardRef, isValidElement, useCallback, useEffect, useRef, useState } from 'react';
+import {
+  Children,
+  cloneElement,
+  forwardRef,
+  isValidElement,
+  useCallback,
+  useEffect,
+  useRef,
+  useState,
+  version as reactVersion
+} from 'react';
 import { MANAGE, MY_VIEWS, SAVE, SAVE_AS, SEARCH, SEARCH_VARIANT, SELECT_VIEW } from '../../i18n/i18n-defaults.js';
 import { stopPropagation } from '../../internal/stopPropagation.js';
 import type { SelectedVariant } from '../../internal/VariantManagementContext.js';
 import { VariantManagementContext } from '../../internal/VariantManagementContext.js';
-import type { ResponsivePopoverDomRef } from '../../webComponents/index.js';
-import {
-  Bar,
-  Button,
-  Icon,
-  IllustratedMessage,
-  Input,
-  List,
-  ResponsivePopover,
-  Title
-} from '../../webComponents/index.js';
+import { Bar } from '../../webComponents/Bar/index.js';
+import { Button } from '../../webComponents/Button/index.js';
+import { Icon } from '../../webComponents/Icon/index.js';
+import { IllustratedMessage } from '../../webComponents/IllustratedMessage/index.js';
+import { Input } from '../../webComponents/Input/index.js';
+import type { ListPropTypes } from '../../webComponents/List/index.js';
+import { List } from '../../webComponents/List/index.js';
+import type { ListItemStandardDomRef } from '../../webComponents/ListItemStandard/index.js';
+import type { ResponsivePopoverDomRef } from '../../webComponents/ResponsivePopover/index.js';
+import { ResponsivePopover } from '../../webComponents/ResponsivePopover/index.js';
+import { Title } from '../../webComponents/Title/index.js';
 import { FlexBox } from '../FlexBox/index.js';
 import type { ManageViewsDialogPropTypes } from './ManageViewsDialog.js';
 import { ManageViewsDialog } from './ManageViewsDialog.js';
 import { SaveViewDialog } from './SaveViewDialog.js';
-import type { VariantManagementPropTypes } from './types.js';
+import type { SelectedVariantWithStringBool, VariantManagementPropTypes } from './types.js';
 import type { VariantItemPropTypes } from './VariantItem.js';
 import { classNames, styleData } from './VariantManagement.module.css.js';
 
+const booleanProps = {
+  favorite: true,
+  global: true,
+  isDefault: true,
+  labelReadOnly: true,
+  applyAutomatically: true,
+  readOnly: true,
+  hideDelete: true
+};
+
 /**
  * The VariantManagement can be used to manage variants (views). You can use this component to create and maintain personalization changes.
  *
  * __Note:__ On the user interface, variants are generally referred to as "views".
  *
+ * __Note:__ Each `VariantManagement` component can only have one default and one selected variant.
+ *
  * ### Matching header styles
  *
  * To ensure consistent header styles for different use-cases of the `VariantManagement`, we recommend setting the following styles to the `ui5-title` component:
@@ -117,14 +139,44 @@ const VariantManagement = forwardRef<HTMLDivElement, VariantManagementPropTypes>
   const [popoverOpen, setPopoverOpen] = useState(false);
   const [manageViewsDialogOpen, setManageViewsDialogOpen] = useState(false);
   const [saveAsDialogOpen, setSaveAsDialogOpen] = useState(false);
-  const [selectedVariant, setSelectedVariant] = useState<SelectedVariant | undefined>(() => {
+  const [selectedVariant, setSelectedVariantState] = useState<SelectedVariant | undefined>(() => {
     const currentSelectedVariant = safeChildren.find(
       (item) => isValidElement(item) && (item as ReactElement<VariantItemPropTypes>).props.selected
     ) as ComponentElement<any, any>;
     if (currentSelectedVariant) {
       return { ...currentSelectedVariant.props, variantItem: currentSelectedVariant.ref };
     }
   });
+  const setSelectedVariant = (variant: SelectedVariantWithStringBool) => {
+    if (variant) {
+      const stringToBoolVariant = Object.entries(variant).reduce((acc, [key, val]) => {
+        if (booleanProps[key]) {
+          if (typeof val === 'boolean') {
+            acc[key] = val;
+            return acc;
+          }
+          if (val === 'false') {
+            acc[key] = false;
+            return acc;
+          }
+          if (val === 'true') {
+            acc[key] = true;
+            return acc;
+          }
+          if (reactVersion.startsWith('19') && val === '') {
+            acc[key] = true;
+            return acc;
+          }
+        }
+        acc[key] = val;
+        return acc;
+      }, {}) as SelectedVariant;
+      setSelectedVariantState(stringToBoolVariant);
+    } else {
+      setSelectedVariantState(variant as SelectedVariant);
+    }
+  };
+
   const [selectedSaveViewInputProps, setSelectedSaveViewInputProps] = useState(
     selectedVariant?.saveViewInputProps ?? {}
   );
@@ -258,8 +310,13 @@ const VariantManagement = forwardRef<HTMLDivElement, VariantManagementPropTypes>
     setSelectedSaveViewInputProps(selectedChild?.props.saveViewInputProps ?? {});
   }, [selectedVariant, safeChildren]);
 
-  const handleVariantItemSelect = (e) => {
-    setSelectedVariant({ ...e.detail.selectedItems[0].dataset, variantItem: e.detail.selectedItems[0] });
+  const handleVariantItemSelect: ListPropTypes['onSelectionChange'] = (e) => {
+    const targetItem = e.detail.targetItem as unknown as ListItemStandardDomRef;
+    const dataset = targetItem.dataset as unknown as SelectedVariantWithStringBool;
+    setSelectedVariant({
+      ...dataset,
+      variantItem: targetItem
+    });
     selectVariantEventRef.current = e;
     if (closeOnItemSelect) {
       handleClose();
@@ -317,19 +374,14 @@ const VariantManagement = forwardRef<HTMLDivElement, VariantManagementPropTypes>
     }
   }, [safeChildrenWithFavorites]);
 
-  //todo: selectedVariant type needs to be enhanced for React19 (data attributes: true => "", false => "false")
-  const showSaveBtn =
-    dirtyState &&
-    selectedVariant &&
-    (typeof selectedVariant?.readOnly === 'string'
-      ? selectedVariant.readOnly !== '' && selectedVariant.readOnly === 'false'
-      : !selectedVariant.readOnly);
+  const showSaveBtn = dirtyState && selectedVariant && !selectedVariant.readOnly;
 
   return (
     <div className={variantManagementClasses} style={style} {...rest} ref={ref}>
       <VariantManagementContext.Provider
         value={{
-          selectVariantItem: setSelectedVariant
+          selectVariantItem: setSelectedVariant,
+          selectedVariant
         }}
       >
         <FlexBox onClick={disabled ? undefined : handleOpenVariantManagement}>
diff --git a/packages/main/src/components/VariantManagement/types.ts b/packages/main/src/components/VariantManagement/types.ts
@@ -13,6 +13,20 @@ import type {
 } from '../../webComponents/index.js';
 import type { VariantItemPropTypes } from './VariantItem.js';
 
+export interface SelectedVariantWithStringBool
+  extends Omit<
+    SelectedVariant,
+    'favorite' | 'global' | 'isDefault' | 'labelReadOnly' | 'applyAutomatically' | 'readOnly' | 'hideDelete'
+  > {
+  favorite?: boolean | string;
+  global?: boolean | string;
+  isDefault?: boolean | string;
+  labelReadOnly?: boolean | string;
+  applyAutomatically?: boolean | string;
+  readOnly?: boolean | string;
+  hideDelete?: boolean | string;
+}
+
 interface UpdatedVariant extends SelectedVariant {
   prevVariant?: VariantItemPropTypes;
 }
diff --git a/packages/main/src/internal/VariantManagementContext.ts b/packages/main/src/internal/VariantManagementContext.ts
@@ -2,10 +2,16 @@ import { createContext } from 'react';
 import type { VariantItemPropTypes } from '../components/VariantManagement/VariantItem.js';
 import type { ListItemStandardDomRef } from '../webComponents/ListItemStandard/index.js';
 
+interface VariantManagementContextTypes {
+  selectVariantItem: (_selectedVariant: SelectedVariant) => void;
+  selectedVariant: SelectedVariant;
+}
+
 export interface SelectedVariant extends VariantItemPropTypes {
   variantItem: ListItemStandardDomRef;
 }
 
-export const VariantManagementContext = createContext({
-  selectVariantItem: (_selectedVariant: SelectedVariant) => {}
+export const VariantManagementContext = createContext<VariantManagementContextTypes>({
+  selectVariantItem: (_selectedVariant: SelectedVariant) => {},
+  selectedVariant: undefined
 });
diff --git a/packages/main/src/internal/withWebComponent.tsx b/packages/main/src/internal/withWebComponent.tsx
@@ -168,7 +168,7 @@ export const withWebComponent = <Props extends Record<string, any>, RefType = Ui
 
     useEffect(() => {
       if (waitForDefine && !isDefined) {
-        customElements.whenDefined(Component as unknown as string).then(() => {
+        void customElements.whenDefined(Component as unknown as string).then(() => {
           setIsDefined(true);
           definedWebComponents.add(Component);
         });
