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

Update "Not Using the <Admin> Components" doc #5058

Merged
merged 2 commits into from
Aug 9, 2020

Conversation

srosset81
Copy link
Contributor

This update the documentation for "Not Using the <Admin> Components" to fix several problems.

  • The example authProvider does not work (const authProvider = () => Promise.resolve();), because authProvider is now supposed to be an object instead of a function.
  • No locale variable is defined.
  • No theme is provided to the <ThemeProvider>.
  • PropTypes does not exist so it must be imported from the prop-types package.
  • The withContext expects authProvider to be a PropTypes.object instead of a PropTypes.func
  • The Notification component is not included, while it is necessary for the data provider to work.

The last problem is probably a bug but I think it can be worth documenting it until the bug is fixed.

+ },
+ () => ({ authProvider })
+)(App);
```

Note that this example still uses `<Resource>`, because this component lazily initializes the store for the resource data.

Currently the `<Notification>` component is required (See [issue #5026](https://github.com/marmelab/react-admin/issues/5026)).
Copy link
Collaborator

Choose a reason for hiding this comment

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

The notification component will always be required for optimistic support so I think you can remove this line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done !
Have to say I find it strange that a component called Notification is required for optimistic update. If it can't be changed, maybe it should be renamed ?

@srosset81 srosset81 requested a review from djhi July 31, 2020 14:23
@fzaninotto fzaninotto merged commit 687fb2d into marmelab:master Aug 9, 2020
@fzaninotto
Copy link
Member

Thanks!

@fzaninotto fzaninotto added this to the 3.8.0 milestone Aug 9, 2020
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