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

Introduce a new AutocompleteInput based on MUI Autocomplete #6924

Merged
merged 27 commits into from Dec 8, 2021

Conversation

djhi
Copy link
Contributor

@djhi djhi commented Dec 2, 2021

  • Avoid breaking change for optionText and leverage the inputText prop we had in previous version

@@ -23,23 +23,8 @@ describe('<AutocompleteInput />', () => {
resource: 'users',
};

it('should use a Downshift', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed because we don't want to test mui autocomplete

@@ -645,37 +632,6 @@ describe('<AutocompleteInput />', () => {
await waitFor(() => expect(queryAllByRole('option').length).toEqual(1));
});

it('passes options.suggestionsContainerProps to the suggestions container', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

mui autocomplete provide its own ways to customize the underlying components

@@ -694,60 +650,6 @@ describe('<AutocompleteInput />', () => {
});
});

it('should not render a LinearProgress if loading is true and a second has not passed yet', () => {
Copy link
Contributor Author

@djhi djhi Dec 3, 2021

Choose a reason for hiding this comment

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

Mui autocomplete has its own loading mechanism. Maybe we should check we don't set its loading prop unless a second has passed

Copy link
Member

Choose a reason for hiding this comment

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

I see that you did it, am I wrong?

@djhi djhi mentioned this pull request Dec 4, 2021
@djhi djhi marked this pull request as ready for review December 6, 2021 11:08
@djhi djhi added the RFR Ready For Review label Dec 6, 2021
@vercel
Copy link

vercel bot commented Dec 6, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

react-admin – ./examples/simple

🔍 Inspect: https://vercel.com/marmelab/react-admin/9hDLVn7GPcfw2AVyC7MJ4wb1NwGv
✅ Preview: https://react-admin-git-mui-autocomplete-marmelab.vercel.app

react-admin-storybook – ./

🔍 Inspect: https://vercel.com/marmelab/react-admin-storybook/AnBNcNU69HiYkbb8qoLuSJUbri5Y
✅ Preview: https://react-admin-storybook-git-mui-autocomplete-marmelab.vercel.app

[Deployment for 0bb5b2e failed]

@fzaninotto
Copy link
Member

I see a UI bug in the Storybook: in the "inside Reference Input" srtory, when typing text, the choices are briefly emptied before being filled with matching choices. I think you should use the keepPreviousData option in useGetList (once it's handled by react-query, of course)

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.

Great! 💯

cypress/integration/edit.js Show resolved Hide resolved
examples/simple/src/comments/CommentEdit.tsx Outdated Show resolved Hide resolved
examples/simple/src/comments/CommentEdit.tsx Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/input/AutocompleteInput.tsx Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/input/SelectArrayInput.tsx Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/input/SelectInput.tsx Outdated Show resolved Hide resolved
@@ -694,60 +650,6 @@ describe('<AutocompleteInput />', () => {
});
});

it('should not render a LinearProgress if loading is true and a second has not passed yet', () => {
Copy link
Member

Choose a reason for hiding this comment

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

I see that you did it, am I wrong?

Copy link
Contributor

@WiXSL WiXSL left a comment

Choose a reason for hiding this comment

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

Editing a Post In simple example:

2021-12-08 082601

I just set a value, and then re-open the AutocompleteInput

UPGRADE.md Outdated Show resolved Hide resolved
@djhi
Copy link
Contributor Author

djhi commented Dec 8, 2021

Editing a Post In simple example:

2021-12-08 082601

I just set a value, and then re-open the AutocompleteInput

Good catch!

Co-authored-by: Aníbal Svarcas <WiXSL@users.noreply.github.com>
UPGRADE.md Outdated Show resolved Hide resolved
Co-authored-by: Aníbal Svarcas <WiXSL@users.noreply.github.com>
@@ -662,17 +667,6 @@ import { AutocompleteInput, ReferenceInput } from 'react-admin';
</ReferenceInput>
```

Lastly, would you need to override the props of the suggestion's container (a `Popper` element), you can specify them using the `options.suggestionsContainerProps`. For example:
Copy link
Contributor

@WiXSL WiXSL Dec 8, 2021

Choose a reason for hiding this comment

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

Since suggestionsContainerProps was introduced by react-admin, shouldn't we clarify how to upgrade this feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's in the MUI documentation, I don't think we should include it

@WiXSL
Copy link
Contributor

WiXSL commented Dec 8, 2021

The vercel deployment doesn't have the authors ArrayInput in Post form and lacks the User menu

@djhi
Copy link
Contributor Author

djhi commented Dec 8, 2021

The vercel deployment doesn't have the authors ArrayInput in Post form and lacks the User menu

You have to log in with admin/password

@WiXSL
Copy link
Contributor

WiXSL commented Dec 8, 2021

I was trying it locally and it caught my attention, you are right

I was trying it locally and it caught my attention, you are right

UPGRADE.md Outdated Show resolved Hide resolved
@fzaninotto fzaninotto merged commit b7e4ed9 into next Dec 8, 2021
@fzaninotto fzaninotto deleted the mui-autocomplete branch December 8, 2021 17:09
@fzaninotto fzaninotto added this to the 4.0.0-alpha.1 milestone Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants