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

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

Closed
rners01 opened this issue Feb 22, 2024 · 16 comments · Fixed by #6271 or #6253
Closed

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

rners01 opened this issue Feb 22, 2024 · 16 comments · Fixed by #6271 or #6253
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@rners01
Copy link

rners01 commented Feb 22, 2024

Describe the bug

<Show isLoading={true}> with Material UI doesn't show loading spinner

Steps To Reproduce

  1. Create view with isLoading prop set to true
  2. Observe no spinner shown

Expected behavior

Spinner on isLoading=true

Packages

"@refinedev/cli": "2.16.24",
"@refinedev/core": "4.47.1",
"@refinedev/kbar": "1.3.6",
"@refinedev/mui": "5.14.4",
"@refinedev/react-hook-form": "4.8.14",
"@refinedev/react-router-v6": "4.5.5",
"@refinedev/simple-rest": "5.0.2",

Additional Context

No response

@rners01 rners01 added the bug Something isn't working label Feb 22, 2024
@rners01 rners01 changed the title [BUG] <Show isLoading={true}> with Material UI doesn't show loader spinner [BUG] <Show isLoading={true}> with Material UI doesn't show loading spinner Feb 22, 2024
@issa012
Copy link
Contributor

issa012 commented Feb 23, 2024

Hello, I tried to recreate this bug. It is working as you said, there is no loading spinner. However, I think that is the expected behavior. When loading, the component doesn't show a loading spinner, but disables the refresh button. I found that here in the documentation:
https://refine.dev/docs/ui-integrations/material-ui/components/basic-views/show/#isloading

@BatuhanW
Copy link
Member

Thanks for the explanation, @issa012. As mentioned you can use this prop to disable refresh button.

@BatuhanW BatuhanW closed this as not planned Won't fix, can't repro, duplicate, stale Feb 28, 2024
@rners01
Copy link
Author

rners01 commented Feb 29, 2024

@BatuhanW @issa012
It's not looking pretty with default values displaying first and then jumping to the API data.
Also Mantine and Ant UI integrations have loading spinner for such scenarios, that's why I though it could be the bug.
So there's no plan to add spinner?

@BatuhanW
Copy link
Member

@rners01 While it's not a bug, we are open to contributions if someone would like to work on this one.

@issa012
Copy link
Contributor

issa012 commented Feb 29, 2024

I can try, if you assign this to me.

@BatuhanW
Copy link
Member

Sure, assigned to you. 🚀

@rners01
Copy link
Author

rners01 commented Feb 29, 2024

@BatuhanW wrong assigned, should've been @issa012 (:

@rners01
Copy link
Author

rners01 commented Apr 12, 2024

@BatuhanW Was the feature merged?

@omeraplak omeraplak reopened this Apr 17, 2024
@omeraplak omeraplak added the good first issue Good for newcomers label Apr 17, 2024
@0xJaskeerat
Copy link

Hey @rners01 @BatuhanW
I would love to try on this issue , can you please assign me ?

Copy link

stale bot commented Jun 18, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Jun 18, 2024
@rners01
Copy link
Author

rners01 commented Jun 19, 2024

@omeraplak Can you assign it to @0xJaskeerat ?

@stale stale bot removed the wontfix This will not be worked on label Jun 19, 2024
@BatuhanW
Copy link
Member

@0xJaskeerat assiged issue to you.

@paoloLigsay
Copy link

I noticed this issue has been stale for a while. I can work on this, could you please assign it to me, @BatuhanW? Thanks!

@aliemir aliemir removed this from the August Release milestone Aug 6, 2024
@aliemir
Copy link
Member

aliemir commented Aug 13, 2024

I'll leave a comment here to help anyone interested to work on this issue about what we want from the implementation. 🙏

This fix needs to be implemented for <Create />, <Edit /> and <Show /> components. isLoading prop should be used to display a spinner overlaying the content.

To avoid any layout shifts, we're recommending using some absolute positioned box inside the <Card /> and positioning the <Card /> as relative.

We can pass sx.position = "relative" but also allow overriding this value like:

    <Card
      {...(wrapperProps ?? {})}
      sx={{
        position: "relative",
        ...wrapperProps?.sx,
      }}
    >

Then we can place a <CircularProgress /> wrapped with <Box /> inside the <Card /> like below:

      {isLoading && (
        <Box
          sx={{
            position: "absolute",
            inset: 0,
            display: "flex",
            justifyContent: "center",
            alignItems: "center",
            zIndex: (theme) => theme.zIndex.drawer + 1,
            // this is needed to support custom themes, dark mode etc.
            bgcolor: (theme) => alpha(theme.palette.background.paper, 0.4),
          }}
        >
          <CircularProgress />
        </Box>
      )}

I'm not sure if this covers all the necessary cases but I guess it will be a good starting point for the implementation.

We'll be happy to help if anyone is interested working on this issue. 🚀

@Anonymous961
Copy link
Contributor

@aliemir is this issue fixed?
image

@aliemir
Copy link
Member

aliemir commented Aug 19, 2024

Hey @Anonymous961, your screenshot is from Ant Design. This issue is related with our Material UI CRUD components and its still looking for contributors 🙏

@aliemir aliemir added this to the September Release milestone Aug 20, 2024
aliemir added a commit that referenced this issue Aug 20, 2024
…/>` components. (#6271) (fixes #5668)

Co-authored-by: Ali Emir Şen <senaliemir@gmail.com>
@BatuhanW BatuhanW linked a pull request Sep 2, 2024 that will close this issue
emrecancorapci pushed a commit to emrecancorapci/refine that referenced this issue Sep 4, 2024
…/>` components. (refinedev#6271) (fixes refinedev#5668)

Co-authored-by: Ali Emir Şen <senaliemir@gmail.com>
emrecancorapci pushed a commit to emrecancorapci/refine that referenced this issue Sep 4, 2024
…/>` components. (refinedev#6271) (fixes refinedev#5668)

Co-authored-by: Ali Emir Şen <senaliemir@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment