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

[Doc] Fix outdated isLoading in Data Provider chapter #10209

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

smeng9
Copy link
Contributor

@smeng9 smeng9 commented Sep 17, 2024

The doc states:

As a consequence, you should always use isPending to determine if you need to show a loading indicator.

It should be consistent with itself.

Probably there are some additional things needs to be discussed and fixed

  1. Some of the stories still uses isLoading
  2. useList still uses isLoading here
    if (isLoading || !data) return;
  3. In some enterprise components we pass isPending to the isLoading prop which is confusing
    return <Timeline isLoading={isPending} records={data} />;

Additional Checks

  • The PR targets master for a bugfix, or next for a feature
  • The PR includes unit tests (if not possible, describe why)
  • The PR includes one or several stories (if not possible, describe why)
  • The documentation is up to date

Also, please make sure to read the contributing guidelines.

Copy link
Collaborator

@djhi djhi left a comment

Choose a reason for hiding this comment

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

Thanks!

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.

Thanks for your PR! And you're right, we have some work left on the documentation.

In useList, checking isLoading is a mistake. Would you like to open a PR to fix it?

Enterprise Edition components have recently been updated to react-admin v5 and should use isPending. The doc about EE components is probably not up to date (it is though at https://react-admin-ee.marmelab.com/whats-new). We'll do a pass to fix the remaining isLoading, but you're welcome to give us a hand!

@fzaninotto fzaninotto merged commit 3369b90 into marmelab:master Sep 17, 2024
13 checks passed
@fzaninotto fzaninotto added this to the 5.2.1 milestone Sep 17, 2024
@smeng9 smeng9 deleted the is-pending branch September 17, 2024 16:50
@fzaninotto fzaninotto changed the title [Doc] Doc needs to be consistent with itself regarding isPending vs isLoading [Doc] Fix outdated isLoading in Data Provider chapter Sep 18, 2024
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