Skip to content

Commit

Permalink
chore!: Use index instead of identifier for cursor navigation (#2194)
Browse files Browse the repository at this point in the history
Virtual lists using dynamic data need to intercept navigation changes to trigger data loading. If data isn't loaded yet, an `id` isn't known yet. The navigation managers were updated to use indexes instead of ids to handle unloaded data. This should not effect most people as the navigation manager is provided by default. This is mostly internal.

[category:Components]

### BREAKING CHANGES
The signature of the `NavigationManaget` and `NavigationRequestor` was changed to use numeric indexes instead of string identifiers. This will only break for those who created a custom navigation manager.
  • Loading branch information
NicholasBoll committed May 3, 2023
1 parent 45ee94e commit 9f388cd
Show file tree
Hide file tree
Showing 5 changed files with 253 additions and 205 deletions.
13 changes: 13 additions & 0 deletions modules/docs/mdx/9.0-UPGRADE-GUIDE.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ any questions.
- [Component Updates](#component-updates)
- [Button](#button)
- [Toast](#toast)
- [Collection](#collection)
- [Utility Updates](#utility-updates)
- [useTheme and getTheme](#usetheme-and-gettheme)
- [useThemedRing](#usethemedring)
Expand Down Expand Up @@ -291,6 +292,8 @@ return {
`as const` instructs Typescript the type is `readonly`. Typescript knows readonly values or objects
cannot be changed and will therefore narrow the type for you.

---

## Token Updates

### Depth
Expand Down Expand Up @@ -378,6 +381,16 @@ previously used `actionText` or `onActionClick`. The codemod will also update im
> **Note:** You will manually need to set `mode` to `alert` if your `Toast` conveys urgent and
> important information such as an error.
---

### Collection

Navigation was updated to use numerical indexes instead of string identifiers. The
`model.state.cursorId` is left unchanged. The change is to support virtual lists where navigation
knows where it needs to go, but the identifier may not be loaded yet. The mechanism for navigating
is private and should not breaking anything. If you created a custom navigation manager, the
signature has been changed.

## Utility Updates

### useTheme and getTheme
Expand Down
36 changes: 24 additions & 12 deletions modules/react/collection/lib/useBaseListModel.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import React from 'react';
import {useVirtual} from './react-virtual';

import {useUniqueId, assert, createModelHook, Generic} from '@workday/canvas-kit-react/common';
import {useUniqueId, createModelHook, Generic} from '@workday/canvas-kit-react/common';

export type Orientation = 'horizontal' | 'vertical';

export const defaultGetId = (item: any): string => {
assert(item.id, 'A list item must have an `id` field or a `getId` function defined');
return item.id;
return item.id || '';
};

export const defaultGetTextValue = (item: any): string => {
Expand Down Expand Up @@ -79,14 +78,18 @@ export const useBaseListModel = createModelHook({
/** IDREF of the list. Children ids can be derived from this id */
id: '',
/**
* Optional function to return an id of an item. If not provided, the default function will return
* the `id` property from the object of each item. If you did not provide `items`, do not override
* this function. If you don't provided `items` and instead provide static items via JSX, the list
* will create an internal array of items where `id` is the only property and the default `getId`
* will return the desired result.
* Optional function to return an id of an item. If not provided, the default function will
* return the `id` property from the object of each item. If you did not provide `items`, do not
* override this function. If you don't provided `items` and instead provide static items via
* JSX, the list will create an internal array of items where `id` is the only property and the
* default `getId` will return the desired result.
*/
getId: (item: Generic) => item.id as string,

/**
* Optional function to return the text representation of an item. If not provided, the default
* function will return the `text` property of the object of each item or an empty string if
* there is no `text` property. If you did not provide `items`, do not override this function.
*/
getTextValue: (item: Generic) => (item.text || '') as string,
/**
* Array of all ids which are currently disabled. This is used for navigation to skip over items
Expand All @@ -112,8 +115,17 @@ export const useBaseListModel = createModelHook({
})(config => {
const id = useUniqueId(config.id);

// Optimization to not redo items when `getId` and `getTextValue` references change. They will not
// likely change during the lifecycle and we don't want to recalculate items when a lamba is
// passed instead of a stable reference.
const getIdRef = React.useRef(defaultGetId);
const getTextValueRef = React.useRef(defaultGetTextValue);

const getId = config.getId || defaultGetId;
const getTextValue = config.getTextValue || defaultGetTextValue;
getIdRef.current = getId;
getTextValueRef.current = getTextValue;

const [orientation] = React.useState(config.orientation || 'vertical');
const [UNSTABLE_defaultItemHeight, setDefaultItemHeight] = React.useState(
config.defaultItemHeight
Expand All @@ -125,13 +137,13 @@ export const useBaseListModel = createModelHook({
() =>
(config.items || []).map((item, index) => {
return {
id: getId(item),
id: getIdRef.current(item),
index,
value: item,
textValue: getTextValue(item),
textValue: getTextValueRef.current(item),
};
}),
[config.items, getId, getTextValue]
[config.items]
);
const [staticItems, setStaticItems] = React.useState<Item<Generic>[]>([]);
const UNSTABLE_virtual = useVirtual({
Expand Down
Loading

0 comments on commit 9f388cd

Please sign in to comment.