Skip to content

Sync admin's docs with the last version (React Admin) #503

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

Merged
merged 4 commits into from
Jun 18, 2018

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Jun 9, 2018

No description provided.

@init-penguin
Copy link

It seems that you've missed authClient={authClient} -> authProvider={authProvider}

@dunglas
Copy link
Member Author

dunglas commented Jun 9, 2018

Good catch @init-penguin, fixed

@init-penguin
Copy link

init-penguin commented Jun 9, 2018

Sorry to point this again, @dunglas, but i think you forgot about the import statement :)
Now it imports the authClient while exporting HydraAdmin with authProvider, thus you should either change the import to import authProvider thing (and filename for consistency too), or change the export part of App.js to authProvider={authClient}

@dunglas
Copy link
Member Author

dunglas commented Jun 9, 2018

I'm not sure we need to update the the filename. The React Admin team changed the name of the property to highlight it can be other things than an API client (I guess), but they haven't updated the name of their own clients (for instance the REST client is still called "restClient"). It's why I've not renamed the file and updated the import. But I'm not sure about this.

@fzaninotto, do you have a hint regarding the naming best practice?

@fzaninotto
Copy link

We try to use "provider" everywhere to avoid confusion. If you found some mentions of restClient, that's a mistake on our side. I'll look into the remaining mentions of pre-2.0 terminology.

@dunglas dunglas merged commit 31607f3 into api-platform:2.2 Jun 18, 2018
@dunglas dunglas deleted the react-admin-2.2 branch June 18, 2018 06:50
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.

3 participants