Skip to content

Commit

Permalink
chore: Collection navigation getItem can return undefined
Browse files Browse the repository at this point in the history
  • Loading branch information
NicholasBoll committed Oct 12, 2024
1 parent 487e829 commit 6239bef
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 29 deletions.
8 changes: 8 additions & 0 deletions modules/docs/mdx/12.0-UPGRADE-GUIDE.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ A note to the reader:
- [Select](#select)
- [Text Area](#text-area)
- [Text Input](#text-input)
- [Collections](#collections)
- [Utility Updates](#utility-updates)
- [Troubleshooting](#troubleshooting)
- [Glossary](#glossary)
Expand Down Expand Up @@ -584,6 +585,13 @@ const theme: PartialEmotionCanvasTheme = {
</CanvasProvider>;
```

### Collections

The `navigation.getItem()` function can now return `undefined` instead of throwing an error when an
item is not found. Throwing an error caused lots of problems when it came to dynamic data. This is a
breaking change if your application uses `getItem`. It will now have to deal with the possibility of
an `undefined`.

## Utility Updates

**PR:** [#2908](https://github.com/Workday/canvas-kit/pull/2908)
Expand Down
13 changes: 7 additions & 6 deletions modules/react/collection/lib/useCursorListModel.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import {assert, createModelHook, Generic} from '@workday/canvas-kit-react/common';
import {createModelHook, Generic} from '@workday/canvas-kit-react/common';

import {useBaseListModel, Item} from './useBaseListModel';

Expand All @@ -18,7 +18,7 @@ export interface NavigationManager {
* and `Ctrl+End` for Grids. */
getLast: NavigationRequestor;
/** Get an item with the provided `id`. */
getItem: (id: string, model: NavigationInput) => Item<Generic>;
getItem: (id: string, model: NavigationInput) => Item<Generic> | undefined;
/** Get the next item after the provided `id`. This will be called when the `Right` arrow key is
* pressed for RTL languages and when the `Left` arrow is pressed for LTR languages. */
getNext: NavigationRequestor;
Expand Down Expand Up @@ -129,10 +129,11 @@ export const getNextPage: NavigationRequestor = (index, {state}) => {
return getLast(index, {state});
};

const getItem: (id: string, model: NavigationInput) => Item<Generic> = (id, {state}) => {
const item = state.items.find(item => item.id === id) || state.items[0]; // no id, return first item
assert(item, `Item not found: ${id}`);
return item;
const getItem: (id: string, model: NavigationInput) => Item<Generic> | undefined = (
id,
{state}
) => {
return state.items.find(item => item.id === id);
};

export const getWrappingOffsetItem =
Expand Down
40 changes: 21 additions & 19 deletions modules/react/collection/lib/useListItemRovingFocus.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,28 +39,30 @@ export const useListItemRovingFocus = createElemPropsHook(useCursorListModel)(
React.useEffect(() => {
if (keyElementRef.current) {
const item = model.navigation.getItem(model.state.cursorId, model);
if (model.state.isVirtualized) {
model.state.UNSTABLE_virtual.scrollToIndex(item.index);
}
if (item) {
if (model.state.isVirtualized) {
model.state.UNSTABLE_virtual.scrollToIndex(item.index);
}

const selector = (id?: string) => {
return document.querySelector<HTMLElement>(`[data-focus-id="${`${id}-${item.id}`}"]`);
};
const selector = (id?: string) => {
return document.querySelector<HTMLElement>(`[data-focus-id="${`${id}-${item.id}`}"]`);
};

// In React concurrent mode, there could be several render attempts before the element we're
// looking for could be available in the DOM
retryEachFrame(() => {
// Attempt to extract the ID from the DOM element. This fixes issues where the server and client
// do not agree on a generated ID
const clientId = keyElementRef.current?.getAttribute('data-focus-id')?.split('-')[0];
const element = selector(clientId) || selector(model.state.id);
// In React concurrent mode, there could be several render attempts before the element we're
// looking for could be available in the DOM
retryEachFrame(() => {
// Attempt to extract the ID from the DOM element. This fixes issues where the server and client
// do not agree on a generated ID
const clientId = keyElementRef.current?.getAttribute('data-focus-id')?.split('-')[0];
const element = selector(clientId) || selector(model.state.id);

element?.focus();
if (element) {
keyElementRef.current = null;
}
return !!element;
}, 5); // 5 should be enough, right?!
element?.focus();
if (element) {
keyElementRef.current = null;
}
return !!element;
}, 5); // 5 should be enough, right?!
}
}
// we only want to run this effect if the cursor changes and not any other time
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ export const useComboboxKeyboardTypeAhead = createElemPropsHook(useComboboxModel
};

const currentItemIndex =
model.state.items.length > 0 ? model.navigation.getItem(model.state.cursorId, model).index : 0;
model.state.items.length > 0
? model.navigation.getItem(model.state.cursorId, model)?.index || 0
: 0;

const handleKeyboardTypeAhead = (key: string, numOptions: number) => {
// If the starting point is beyond the list of options, reset it
Expand Down
4 changes: 2 additions & 2 deletions modules/react/select/lib/hooks/useSelectInput.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export const useSelectInput = composeHooks(
model.state.items.length > 0 &&
model.state.selectedIds[0]
) {
const value = model.navigation.getItem(model.state.selectedIds[0], model).id;
const value = model.state.selectedIds[0];
const oldValue = model.state.inputRef.current.value;

// force the hidden input to have the correct value
Expand Down Expand Up @@ -184,7 +184,7 @@ export const useSelectInput = composeHooks(
onChange: noop,
value:
model.state.selectedIds.length > 0 && model.state.items.length > 0
? model.navigation.getItem(model.state.selectedIds[0], model).textValue
? model.navigation.getItem(model.state.selectedIds[0], model)?.textValue || ''
: '',
},
'aria-haspopup': 'menu',
Expand Down
2 changes: 1 addition & 1 deletion modules/react/select/stories/examples/WithIcons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export const WithIcons = () => {
<FormField.Input
as={Select.Input}
cs={styleOverrides.formfieldInputStyles}
inputStartIcon={selectedItem.value.icon}
inputStartIcon={selectedItem?.value.icon}
/>
<Select.Popper>
<Select.Card cs={styleOverrides.selectCardStyles}>
Expand Down

0 comments on commit 6239bef

Please sign in to comment.