-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFR] Introduce useGetMatching and useReferenceArrayInputController #3698
Conversation
djhi
commented
Sep 16, 2019
•
edited
Loading
edited
- useGetMatching
- useReferenceArrayInputController
- Fix tests
3db9430
to
f84e6cb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Encouraging!
packages/ra-core/src/controller/input/ReferenceArrayInputController.spec.tsx
Outdated
Show resolved
Hide resolved
packages/ra-core/src/controller/input/ReferenceArrayInputController.spec.tsx
Show resolved
Hide resolved
packages/ra-core/src/controller/input/ReferenceArrayInputController.spec.tsx
Outdated
Show resolved
Hide resolved
const useReferenceArrayInputController = ({ | ||
basePath, | ||
filter: defaultFilter, | ||
filterToQuery = searchText => ({ q: searchText }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the default value to a constant to avoid having a new function at every render, defeating your useMemo line 93
packages/ra-core/src/controller/input/useReferenceArrayInputController.ts
Show resolved
Hide resolved
options | ||
); | ||
|
||
const finalReferenceRecords = referenceRecords.filter( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a comment here, like "filter out not found references - happens when the dataProvider doesn't guarantee referential integrity"
|
||
const dataStatus = getDataStatus({ | ||
input, | ||
// We merge the currently selected records with the matching ones, otherwise |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should move this comment up to line 115
const referenceSource = (resource, source) => `${resource}@${source}`; | ||
|
||
/** | ||
* Call the dataProvider with a GET_LIST verb and return the result as well as the loading state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment is the same as useGetList... You should tweak it so that readers understand the difference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is tweaked in the example. GET_MATCHING is actually sent as GET_LIST
* | ||
* import { useGetMatching } from 'react-admin'; | ||
* | ||
* const LatestNews = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad example