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

feat(mui): add a loading spinner to <Show> component #6131

Conversation

paoloLigsay
Copy link

PR Checklist

Please check if your PR fulfills the following requirements:

Bugs / Features

What is the current behavior?

The <Show> component in the @refinedev/mui package currently does not display a loading spinner when data is being fetched or processed, leaving users without visual feedback during loading states.

What is the new behavior?

The <Show> component now includes a loading spinner that appears when the isLoading prop is set to true. This provides users with a visual indication that data is being loaded during loading states. A test has been added to verify that the loading spinner renders when the isLoading prop is set to true.

fixes #5668

Notes for reviewers

The loading spinner added to the <Show> component is based on the behavior and color of the spinner used in the <List> component. It utilizes the <CircularProgress> component with its color set to theme.palette.primary.main.

See testing when isLoading is set to True:

when-loading-is-true.mov

See testing of the actual behavior:

actual-loading.mov

This is what the current loading spinner looks like in the <List> component:

comparison-with-list-load.mov

@paoloLigsay paoloLigsay requested a review from a team as a code owner July 12, 2024 03:54
Copy link

changeset-bot bot commented Jul 12, 2024

🦋 Changeset detected

Latest commit: 16a9d13

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@refinedev/mui Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@alicanerdurmaz
Copy link
Member

thanks for the detailed explanation 🚀

@alicanerdurmaz alicanerdurmaz added this to the August Release milestone Jul 12, 2024
@BatuhanW BatuhanW changed the base branch from master to releases/august July 17, 2024 10:59
@alicanerdurmaz
Copy link
Member

Hello again @paoloLigsay, the PR is great, thank you for your effort. However, after discussing with the core team, we decided to make some improvements regarding the design.

The loading should be an overlay, ensuring that the children of the <Show /> component remain visible and not unmounted, similar to the <Show /> component of Ant Design. We thought this way it would prevent flicker and look better.

image
Implementing this design might require custom CSS, but it's worth it.

Also, we need to add the same features to the <List /> and <Edit /> components.

Do you want to continue to work on this?

@aliemir aliemir deleted the branch refinedev:master August 5, 2024 10:32
@aliemir aliemir closed this Aug 5, 2024
@aliemir aliemir reopened this Aug 6, 2024
@aliemir aliemir changed the base branch from releases/august to master August 6, 2024 14:19
@aliemir aliemir removed this from the August Release milestone Aug 6, 2024
@aliemir
Copy link
Member

aliemir commented Aug 13, 2024

Unfortunately this PR is left incomplete and we're closing this due to inactivity. If anyone is interested on working on this implementation, please let us know in the related issue 🙏

@aliemir aliemir closed this Aug 13, 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.

[BUG] <Show isLoading={true}> with Material UI doesn't show loading spinner
3 participants