Chore(ArrayField): add support for `storeKey` to manage independent selection states by Aijeyomah · Pull Request #10390 · marmelab/react-admin

Aijeyomah · 2024-12-01T22:33:31Z

Problem

#10382
Describe the problem this PR solves

Solution

Introduced a storeKey property for ArrayField to enable distinct selection state storage for multiple instances of the same data source.
Added documentation and examples to demonstrate usage.

https://www.loom.com/share/b6da1650760d4a83981f602341b667e7?sid=3fa2812e-1cdc-4266-9958-89c9ff731657

Describe the solution this PR implements

How To Test

Describe the steps required to test the changes

Additional Checks

The PR targets master for a bugfix, or next for a feature
The PR includes unit tests (if not possible, describe why)
The PR includes one or several stories (if not possible, describe why)
The documentation is up to date

Also, please make sure to read the contributing guidelines.

Aijeyomah · 2024-12-04T00:56:28Z

@fzaninotto Can you review? thanks

djhi · 2024-12-04T08:15:10Z

Thanks for the PR! As this is a new feature, can you target the next branch? Besides, in order to test it, we'll need stories and unit tests. Can you add some, taking inspiration from the ListBase component?

Aijeyomah · 2024-12-04T09:43:51Z

Thanks for the PR! As this is a new feature, can you target the next branch? Besides, in order to test it, we'll need stories and unit tests. Can you add some, taking inspiration from the ListBase component?

Yes, I can. Thanks

Aijeyomah · 2025-01-02T13:36:51Z

Thanks for the PR! As this is a new feature, can you target the next branch? Besides, in order to test it, we'll need stories and unit tests. Can you add some, taking inspiration from the ListBase component?

@djhi @fzaninotto I have updated the target branch as well as written the necessary stories and tests for the feature. Please re-review. Thanks

slax57

Too bad you took a wrong direction with the fix, but we appreciate the effort to provide good stories, tests, and documentation! Thanks!

slax57 · 2025-01-07T16:36:56Z

packages/ra-core/src/controller/list/useList.ts

+    useEffect(() => {
+        if (!resource) return;
+        // Check if storeKey exists in the pagination store
+        const currentPagination = storeKeyPaginationRef.current[resource];
+        if (currentPagination) {
+            // Restore existing pagination state for the storeKey
+            if (
+                page !== currentPagination.page ||
+                perPage !== currentPagination.perPage
+            ) {
+                setPage(currentPagination.page);
+                setPerPage(currentPagination.perPage);
+            }
+        } else {
+            setPage(initialPage);
+            setPerPage(initialPerPage);
+        }
+        storeKeyPaginationRef.current[resource] = { page, perPage };
+    }, [
+        resource,
+        setPage,
+        setPerPage,
+        initialPage,
+        initialPerPage,
+        page,
+        perPage,
+    ]);
+


I'm afraid you are going in the wrong direction here 😬 . The fix for #10382 suggested by @fzaninotto was to allow to override the storeKey used to store the selection state (i.e. used in useRecordSelection), but not the pagination state.

You can have a look at what was done in useReferenceManyFieldController for an example:

react-admin/packages/ra-core/src/controller/field/useReferenceManyFieldController.ts

Lines 90 to 92 in 7b319c4

const [selectedIds, selectionModifiers] = useRecordSelection({

resource: storeKey,

});

slax57 · 2025-01-07T16:40:30Z

packages/ra-core/src/controller/list/useList.storeKey.stories.tsx

+export const ListsWithoutStoreKeys = () => (
+    <CoreAdminContext store={localStorageStore()} dataProvider={dataProvider}>
+        <CoreAdminUI layout={Layout}>
+            <CustomRoutes>
+                <Route path="/store" element={StorePosts} />
+                <Route path="/nostore" element={NoStorePosts} />
+            </CustomRoutes>
+            <Resource name="posts" />
+        </CoreAdminUI>
+    </CoreAdminContext>
+);


This Story is a bit misleading, StorePosts is still using a storeKey.

Besides, storeKey={false} is not the same as having no storeKey (for NoStorePosts ). I'd expect both lists in ListsWithoutStoreKeys to have no storeKey.

slax57 · 2025-01-07T16:42:44Z

packages/ra-core/src/controller/list/useList.storeKey.stories.tsx

+const Layout = (props: CoreLayoutProps) => (
+    <div style={styles.mainContainer}>
+        <Link aria-label="top" to={`/top`}>
+            Go to Top Posts
+        </Link>
+        <Link aria-label="flop" to={`/flop`}>
+            Go to Flop Posts
+        </Link>
+        <Link aria-label="store" to={`/store`}>
+            Go to Store List
+        </Link>
+        <Link aria-label="nostore" to={`/nostore`}>
+            Go to No-Store List
+        </Link>
+
+        <br />
+        {props.children}
+    </div>
+);


Layout should not be shared across stories since both stories do not implement the same routes

slax57 · 2025-01-07T16:45:23Z

docs/ArrayField.md

+
+When displaying multiple lists with the same data source, you may need to distinguish their selection states. To achieve this, assign a unique `storeKey` to each `ArrayField`. This allows each list to maintain its own selection state independently.
+
+In the example below, two `ArrayField` components display the same data source (`books`), but each stores its selection state under a different key (`books.selectedIds` and `custom.selectedIds`). This ensures that both components can coexist on the same page without interfering with each other's state.


Suggested change

In the example below, two `ArrayField` components display the same data source (`books`), but each stores its selection state under a different key (`books.selectedIds` and `custom.selectedIds`). This ensures that both components can coexist on the same page without interfering with each other's state.

In the example below, two `ArrayField` components display the same data source (`books`), but each stores its selection state under a different key (`customOne.selectedIds` and `customTwo.selectedIds`). This ensures that both components can coexist on the same page without interfering with each other's state.

Aijeyomah · 2025-03-17T00:17:47Z

@slax57 Can you re-review? Thanks.

fzaninotto · 2025-03-17T08:49:39Z

packages/ra-ui-materialui/src/field/ArrayField.spec.tsx

+    jest.spyOn(console, 'error').mockImplementation((message, ...args) => {
+        if (
+            typeof message === 'string' &&
+            message.includes('React will try recreating this component tree')


This isn't a normal error message - it probably comes from your story, your test, or your implementation.

fzaninotto · 2025-03-17T08:50:16Z

packages/ra-ui-materialui/src/field/ArrayField.spec.tsx

-import { ListContext } from './ArrayField.stories';
+import { ListContext, TwoArrayFieldsSelection } from './ArrayField.stories';
+
+beforeAll(() => {


IMHO, this is a footgun as it will hide potentially important errors. I'd prefer that you do the mock in individual tests and only when necessary

fzaninotto · 2025-03-17T08:52:34Z

packages/ra-ui-materialui/src/field/ArrayField.stories.tsx

+                                <Datagrid
+                                    rowClick="toggleSelection"
+                                    bulkActionButtons={true}
+                                    isRowSelectable={() => true}


Rows are selectable by default, you shouldn't need this line. Same for the second ArrayInput.

fzaninotto · 2025-03-17T08:53:53Z

packages/ra-ui-materialui/src/field/ArrayField.spec.tsx

+
+        const checkboxes = screen.queryAllByRole('checkbox');
+
+        expect(checkboxes.length).toBeGreaterThan(3);


By reading the test, I don't know why you wait for that. If you want to wait for the two lists to render, please use a findByText looking for the actual array values instead.

fzaninotto · 2025-03-17T08:54:44Z

packages/ra-ui-materialui/src/field/ArrayField.spec.tsx

+        expect(checkboxes.length).toBeGreaterThan(3);
+
+        // Select an item in the memberships list
+        fireEvent.click(checkboxes[1]);


Since you enabled rowClick, I suggest you trigger a click on a piece of text instead. This would make the test much more readable as it's not obvious what checkboxes[1] is.

fzaninotto · 2025-03-17T08:54:59Z

packages/ra-ui-materialui/src/field/ArrayField.spec.tsx

+
+        // Select an item in the memberships list
+        fireEvent.click(checkboxes[1]);
+        render(<TwoArrayFieldsSelection />);


You shouldn't need to rerender. If you want to come back to the initial state, use more than one it() and group them in a describe().

fzaninotto · 2025-03-17T08:58:09Z

packages/ra-ui-materialui/src/field/ArrayField.tsx

-    const listContext = useList({ data, resource, perPage, sort, filter });
+    const listContext = useList({
+        data,
+        resource: storeKey || resource, // Prioritize storeKey if provided


I'd prefer that you add support for a custom storeKey in useList. I know that we currently use resource only for selection in useList, but this may change in the future.

vercel bot deployed to Preview – react-admin-storybook December 1, 2024 22:35 View deployment

Aijeyomah changed the title ~~chore: add storeKey to arrayfield to handle resources with same ids~~ **Chore(ArrayField): add support for storeKey to manage independent selection states** Dec 1, 2024

Aijeyomah changed the title ~~**Chore(ArrayField): add support for storeKey to manage independent selection states**~~ Chore(ArrayField): add support for storeKey to manage independent selection states Dec 1, 2024

vercel bot deployed to Preview – react-admin-storybook December 1, 2024 23:14 View deployment

Aijeyomah changed the base branch from master to next December 17, 2024 10:02

Aijeyomah force-pushed the add-storekey-to-arrayfield branch from d8c1f4f to d852f0c Compare January 2, 2025 13:32

vercel bot deployed to Preview – react-admin-storybook January 2, 2025 13:34 View deployment

slax57 requested changes Jan 7, 2025

View reviewed changes

Aijeyomah force-pushed the add-storekey-to-arrayfield branch from d852f0c to 46a45d8 Compare March 16, 2025 23:05

vercel bot deployed to Preview – react-admin-storybook March 16, 2025 23:09 View deployment

Aijeyomah force-pushed the add-storekey-to-arrayfield branch 2 times, most recently from 4b92f80 to 924b2d5 Compare March 16, 2025 23:22

vercel bot deployed to Preview – react-admin-storybook March 16, 2025 23:27 View deployment

Aijeyomah force-pushed the add-storekey-to-arrayfield branch from 924b2d5 to 3a259c9 Compare March 16, 2025 23:29

vercel bot deployed to Preview – react-admin-storybook March 16, 2025 23:33 View deployment

Aijeyomah force-pushed the add-storekey-to-arrayfield branch from 3a259c9 to dc04998 Compare March 16, 2025 23:45

Aijeyomah closed this Mar 16, 2025

Aijeyomah force-pushed the add-storekey-to-arrayfield branch from dc04998 to 89d445b Compare March 16, 2025 23:46

Isolate selection in multiple ArrayFields using storeKey

1aa6b00

Aijeyomah reopened this Mar 16, 2025

vercel bot deployed to Preview – react-admin-storybook March 16, 2025 23:55 View deployment

add instruction on the usage of storeKey in ArrayFIeld docs

21dbb88

vercel bot deployed to Preview – react-admin-storybook March 17, 2025 00:01 View deployment

fix lint

91327b6

Aijeyomah requested a review from slax57 March 17, 2025 00:14

vercel bot deployed to Preview – react-admin-storybook March 17, 2025 00:14 View deployment

add test for array field with storekey

5949c23

vercel bot deployed to Preview – react-admin-storybook March 17, 2025 01:08 View deployment

fzaninotto requested changes Mar 17, 2025

View reviewed changes

	const [selectedIds, selectionModifiers] = useRecordSelection({
	resource: storeKey,
	});


		When displaying multiple lists with the same data source, you may need to distinguish their selection states. To achieve this, assign a unique `storeKey` to each `ArrayField`. This allows each list to maintain its own selection state independently.

		In the example below, two `ArrayField` components display the same data source (`books`), but each stores its selection state under a different key (`books.selectedIds` and `custom.selectedIds`). This ensures that both components can coexist on the same page without interfering with each other's state.


		const checkboxes = screen.queryAllByRole('checkbox');

		expect(checkboxes.length).toBeGreaterThan(3);

Uh oh!

Conversation

Aijeyomah commented Dec 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

How To Test

Additional Checks

Uh oh!

Aijeyomah commented Dec 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

djhi commented Dec 4, 2024

Uh oh!

Aijeyomah commented Dec 4, 2024

Uh oh!

Aijeyomah commented Jan 2, 2025

Uh oh!

slax57 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aijeyomah commented Mar 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Aijeyomah commented Dec 1, 2024 •

edited

Loading

Aijeyomah commented Dec 4, 2024 •

edited

Loading