diff --git a/src/collection-preferences/content-display/__tests__/content-display.test.tsx b/src/collection-preferences/content-display/__tests__/content-display.test.tsx index d934344498..37d7e83250 100644 --- 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([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 index 2b3ef787df..94d4fb52ae 100644 --- 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 index f4ee0da4bb..1949803a79 100644 --- 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 index 7939fe6cb2..9877ce3ed6 100644 --- 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; contentDisplay: ReadonlyArray; }): ReadonlyArray { - const optionsById: Record = 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(); + // 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, - filterText: string -) { +export function getFilteredOptions(options: ReadonlyArray, filterText: string) { filterText = filterText.trim().toLowerCase(); if (!filterText) {