Skip to content
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] Use reference input #3313

Merged
merged 48 commits into from
Jul 22, 2019
Merged

[RFR] Use reference input #3313

merged 48 commits into from
Jul 22, 2019

Conversation

ThieryMichel
Copy link
Contributor

No description provided.

@djhi djhi added this to the 3.0.0 milestone Jun 10, 2019
@ThieryMichel ThieryMichel force-pushed the useReferenceInput branch 2 times, most recently from e650393 to 2d63f10 Compare July 1, 2019 11:32
@ThieryMichel ThieryMichel changed the title [WIP] Use reference input [RFR] Use reference input Jul 1, 2019
@ThieryMichel ThieryMichel changed the title [RFR] Use reference input [WIP] Use reference input Jul 1, 2019
@ThieryMichel ThieryMichel changed the title [WIP] Use reference input [RFR] Use reference input Jul 4, 2019
packages/ra-core/src/controller/useReference.spec.ts Outdated Show resolved Hide resolved
packages/ra-core/src/controller/usePaginationState.spec.ts Outdated Show resolved Hide resolved
packages/ra-core/src/controller/usePaginationState.spec.ts Outdated Show resolved Hide resolved
packages/ra-core/src/controller/usePaginationState.spec.ts Outdated Show resolved Hide resolved
packages/ra-core/src/controller/useFilterState.spec.ts Outdated Show resolved Hide resolved
@ThieryMichel ThieryMichel force-pushed the useReferenceInput branch 2 times, most recently from 3548e8d to d11e9ed Compare July 10, 2019 15:00
Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, almost there! Love the unit test coverage, great job on that.

@@ -922,7 +922,7 @@ The child component may further filter results (that's the case, for instance, f

The child component receives the following props from `<ReferenceInput>`:

- `isLoading`: whether the request for possible values is loading or not
- `loading`: whether the request for possible values is loading or not
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a BC break ; please add a migration guide in the UPDATE.md file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And btw, why renaming it ?

@@ -64,26 +55,16 @@ export interface UseReferenceProps {
*
* @returns {ReferenceProps} The reference props
*/
export const useReference = ({
allowEmpty = false,
export const getResourceLinkPath = ({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to export it, it's the default export

* @property {Object} referenceRecord: the referenced record.
* @property {string | false} resourceLinkPath link to the page of the related record (depends on link) (false is no link)
*/

/**
* Fetch reference record, and return it when avaliable
* Fetch reference record, and return it when available also provide link toward the referenced resource
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad jsDoc, it is now getResourceLinkPath

...props
}) => {
return children({
...props,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's weird to pass the props there, in other controllers (e.g. useEditController), it's the caller's job to pass the initial props

},
});

rerender(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should not be in this test, which says 'on mount'

* source: 'post_id',
* });
*
* The hook alos allow to filter results. it returns a `setFilter`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* The hook alos allow to filter results. it returns a `setFilter`
* The hook also allow to filter results. It returns a `setFilter`

@fzaninotto
Copy link
Member

Also, needs rebase

ThieryMichel and others added 22 commits July 18, 2019 20:48
…ec.tsx

Co-Authored-By: Francois Zaninotto <francois@marmelab.com>
…ec.tsx

Co-Authored-By: Francois Zaninotto <francois@marmelab.com>
…ec.tsx

Co-Authored-By: Francois Zaninotto <francois@marmelab.com>
…ec.tsx

Co-Authored-By: Francois Zaninotto <francois@marmelab.com>
…ec.tsx

Co-Authored-By: Francois Zaninotto <francois@marmelab.com>
…ec.tsx

Co-Authored-By: Francois Zaninotto <francois@marmelab.com>
….spec.tsx

Co-Authored-By: Francois Zaninotto <francois@marmelab.com>
Co-Authored-By: Francois Zaninotto <francois@marmelab.com>
UPGRADE.md Outdated

## ReferenceInputController isLoading injected props renamed to loading

When using custom component with ReferenceInputController, you should rename the component `isLoading` props to `loading`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/props/prop/

@@ -922,7 +922,7 @@ The child component may further filter results (that's the case, for instance, f

The child component receives the following props from `<ReferenceInput>`:

- `isLoading`: whether the request for possible values is loading or not
- `loading`: whether the request for possible values is loading or not
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And btw, why renaming it ?

As requested by djhi at #3313 (comment)
@fzaninotto
Copy link
Member

And btw, why renaming it ?

Because I asked for it, for consistency with the dataProvider hooks.

@fzaninotto fzaninotto merged commit 7902574 into next Jul 22, 2019
@fzaninotto fzaninotto deleted the useReferenceInput branch July 22, 2019 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants