fix: Display all options for CollectionPreferences content display (#…

…2807) Co-authored-by: Gethin Webster <[email protected]> Co-authored-by: Gethin Webster <[email protected]>
cloudscape-design · Oct 18, 2024 · b69c9b2 · b69c9b2
1 parent b8d559d
commit b69c9b2
Show file tree

Hide file tree

Showing 4 changed files with 88 additions and 27 deletions.
diff --git a/src/collection-preferences/content-display/__tests__/content-display.test.tsx b/src/collection-preferences/content-display/__tests__/content-display.test.tsx
@@ -43,6 +43,11 @@ describe('Content Display preference', () => {
         testOption({ wrapper, option: options[i], index: i });
       }
     });
+
+    it('renders all options even if a partial list is provided in preferences', () => {
+      const wrapper = renderContentDisplay({ preferences: { contentDisplay: [{ id: 'id1', visible: true }] } });
+      expect(wrapper.findOptions()).toHaveLength(4);
+    });
   });
 
   describe('Content visibility', () => {
@@ -86,6 +91,12 @@ describe('Content Display preference', () => {
       expect(toggleInput.getElement()).toBeChecked();
       expect(toggleInput.getElement()).toBeDisabled();
     });
+
+    it('allows toggling options that are not part of the current preferences', () => {
+      const wrapper = renderContentDisplay({ preferences: { contentDisplay: [{ id: 'id1', visible: true }] } });
+      wrapper.findOptionByIndex(3)!.findVisibilityToggle().findNativeInput().click();
+      expectVisibilityStatus({ wrapper, statuses: [true, false, true, false] });
+    });
   });
 
   describe('Content reordering', () => {
@@ -173,6 +184,32 @@ describe('Content Display preference', () => {
       testOrder({ wrapper, order: [0, 1, 2, 3] });
       await expectAnnouncement(wrapper, 'Reordering canceled');
     });
+
+    it('moves item between options that are not part of the current preferences', async () => {
+      const wrapper = renderContentDisplay({ preferences: { contentDisplay: [{ id: 'id1', visible: true }] } });
+      const dragHandle = wrapper.findOptionByIndex(1)!.findDragHandle().getElement();
+      pressKey(dragHandle, 'Space');
+      await expectAnnouncement(wrapper, 'Picked up item at position 1 of 4');
+      pressKey(dragHandle, 'ArrowDown');
+      await expectAnnouncement(wrapper, 'Moving item to position 2 of 4');
+      pressKey(dragHandle, 'ArrowDown');
+      await expectAnnouncement(wrapper, 'Moving item to position 3 of 4');
+      pressKey(dragHandle, 'Space');
+      testOrder({ wrapper, order: [1, 2, 0, 3] });
+      await expectAnnouncement(wrapper, 'Item moved from position 1 to position 3 of 4');
+    });
+
+    it('moves an item not part of the current preferences above the ones that are', async () => {
+      const wrapper = renderContentDisplay({ preferences: { contentDisplay: [{ id: 'id1', visible: true }] } });
+      const dragHandle = wrapper.findOptionByIndex(2)!.findDragHandle().getElement();
+      pressKey(dragHandle, 'Space');
+      await expectAnnouncement(wrapper, 'Picked up item at position 2 of 4');
+      pressKey(dragHandle, 'ArrowUp');
+      await expectAnnouncement(wrapper, 'Moving item to position 1 of 4');
+      pressKey(dragHandle, 'Space');
+      testOrder({ wrapper, order: [1, 0, 2, 3] });
+      await expectAnnouncement(wrapper, 'Item moved from position 2 to position 1 of 4');
+    });
   });
 
   describe.each<boolean>([false, true])('Filtering (with i18n = %s)', withI18nProvider => {

diff --git a/src/collection-preferences/content-display/__tests__/utils.test.ts b/src/collection-preferences/content-display/__tests__/utils.test.ts
@@ -50,4 +50,24 @@ describe('getSortedOptions', () => {
       },
     ]);
   });
+
+  it('adds options not in the state at the end of the list', () => {
+    const options = [
+      { id: 'a', label: 'a' },
+      { id: 'b', label: 'b' },
+      { id: 'c', label: 'c' },
+      { id: 'd', label: 'd' },
+    ];
+    const contentDisplay = [
+      { id: 'c', visible: false },
+      { id: 'b', visible: true },
+    ];
+    const result = getSortedOptions({ options, contentDisplay });
+    expect(result).toEqual([
+      { id: 'c', label: 'c', visible: false },
+      { id: 'b', label: 'b', visible: true },
+      { id: 'a', label: 'a', visible: false },
+      { id: 'd', label: 'd', visible: false },
+    ]);
+  });
 });
diff --git a/src/collection-preferences/content-display/index.tsx b/src/collection-preferences/content-display/index.tsx
@@ -52,17 +52,20 @@ export default function ContentDisplayPreference({
   const i18n = useInternalI18n('collection-preferences');
   const [columnFilteringText, setColumnFilteringText] = useState('');
 
-  const onToggle = (option: OptionWithVisibility) => {
-    onChange(value.map(item => (item.id === option.id ? { ...item, visible: !option.visible } : item)));
-  };
-
   const titleId = `${idPrefix}-title`;
   const descriptionId = `${idPrefix}-description`;
 
-  const sortedAndFilteredOptions = useMemo(
-    () => getFilteredOptions(getSortedOptions({ options, contentDisplay: value }), columnFilteringText),
-    [columnFilteringText, options, value]
-  );
+  const [sortedOptions, sortedAndFilteredOptions] = useMemo(() => {
+    const sorted = getSortedOptions({ options, contentDisplay: value });
+    const filtered = getFilteredOptions(sorted, columnFilteringText);
+    return [sorted, filtered];
+  }, [columnFilteringText, options, value]);
+
+  const onToggle = (option: OptionWithVisibility) => {
+    // We use sortedOptions as base and not value because there might be options that
+    // are not in the value yet, so they're added as non-visible after the known ones.
+    onChange(sortedOptions.map(({ id, visible }) => ({ id, visible: id === option.id ? !option.visible : visible })));
+  };
 
   const { activeItem, collisionDetection, handleKeyDown, sensors, setActiveItem } = useDragAndDropReorder({
     sortedOptions: sortedAndFilteredOptions,
@@ -93,7 +96,7 @@ export default function ContentDisplayPreference({
       'contentDisplayPreference.liveAnnouncementDndDiscarded',
       liveAnnouncementDndDiscarded
     ),
-    sortedOptions: value,
+    sortedOptions: sortedAndFilteredOptions,
   });
 
   const renderedDragHandleAriaDescription = i18n(
@@ -176,9 +179,10 @@ export default function ContentDisplayPreference({
           const { active, over } = event;
 
           if (over && active.id !== over.id) {
-            const oldIndex = value.findIndex(({ id }) => id === active.id);
-            const newIndex = value.findIndex(({ id }) => id === over.id);
-            onChange(arrayMove([...value], oldIndex, newIndex));
+            const oldIndex = sortedOptions.findIndex(({ id }) => id === active.id);
+            const newIndex = sortedOptions.findIndex(({ id }) => id === over.id);
+            // We need to remember to trim the options down to id and visible to emit changes.
+            onChange(arrayMove([...sortedOptions], oldIndex, newIndex).map(({ id, visible }) => ({ id, visible })));
           }
         }}
         onDragCancel={() => setActiveItem(null)}

diff --git a/src/collection-preferences/content-display/utils.ts b/src/collection-preferences/content-display/utils.ts
@@ -3,7 +3,7 @@
 import { CollectionPreferencesProps } from '../interfaces';
 
 export interface OptionWithVisibility extends CollectionPreferencesProps.ContentDisplayOption {
-  visible?: boolean;
+  visible: boolean;
 }
 
 export function getSortedOptions({
@@ -13,22 +13,22 @@ export function getSortedOptions({
   options: ReadonlyArray<CollectionPreferencesProps.ContentDisplayOption>;
   contentDisplay: ReadonlyArray<CollectionPreferencesProps.ContentDisplayItem>;
 }): ReadonlyArray<OptionWithVisibility> {
-  const optionsById: Record<string, CollectionPreferencesProps.ContentDisplayOption> = options.reduce(
-    (currentValue, option) => ({ ...currentValue, [option.id]: option }),
-    {}
-  );
-  return contentDisplay
-    .map(({ id, visible }: CollectionPreferencesProps.ContentDisplayItem) => ({
-      ...optionsById[id],
-      visible,
-    }))
-    .filter(Boolean);
+  // By using a Map, we are guaranteed to preserve insertion order on future iteration.
+  const optionsById = new Map<string, OptionWithVisibility>();
+  // We insert contentDisplay first so we respect the currently selected order
+  for (const { id, visible } of contentDisplay) {
+    // If an option is provided in contentDisplay and not options, we default the label to the id
+    optionsById.set(id, { id, label: id, visible });
+  }
+  // We merge options data, and insert any that were not in contentDisplay as non-visible
+  for (const option of options) {
+    const existing = optionsById.get(option.id);
+    optionsById.set(option.id, { ...option, visible: !!existing?.visible });
+  }
+  return Array.from(optionsById.values());
 }
 
-export function getFilteredOptions(
-  options: ReadonlyArray<CollectionPreferencesProps.ContentDisplayOption>,
-  filterText: string
-) {
+export function getFilteredOptions(options: ReadonlyArray<OptionWithVisibility>, filterText: string) {
   filterText = filterText.trim().toLowerCase();
 
   if (!filterText) {