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

Add support for a custom loading component #246

Merged

Conversation

joseph-bayer
Copy link
Contributor

Hey there!

This is a pull request to add support for replacing the default loading UI ("Loading") with a React component. The idea is from this issue: #245

I had to use React.ComponentType as opposed to the more flexible ReactNode because ReactNode caused an infinite loop in the deepMerge function.

In the issue thread, it was mentioned that a new default loading state would be nice. I'm open to implementing that! For example, we could do some sort of skeleton. If we do implement a skeleton, let me know how detailed it should be (big rectangle vs lots of smaller shapes).

@mathewjordan
Copy link
Member

mathewjordan commented Feb 6, 2025

Hey @joseph-bayer I'm going to review this as-is but perhaps we can still work towards updating the default with a skeleton? I image something very simple that grows and conforms with the general layout of Clover. The only large rectangle would be the canvas where images/video are rendered, something giving the impression of rich content.

██████████████

███████████████████████   █████████
███████████████████████   █████  
███████████████████████   ███████   
███████████████████████   ████████ 
███████████████████████   
███████████████████████

Related but not blocking changes is getting Clover fully working with Radix UI Themes which has a Skeleton component https://www.radix-ui.com/themes/docs/components/skeleton. This work is being completed in #202 but is a bit stale right now.

Copy link
Member

@mathewjordan mathewjordan left a comment

Choose a reason for hiding this comment

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

This looks great and functional. Thanks for addition.

Can we also add a section in the docs in viewer.mdx for with a simple example?

<Viewer
  iiifContent="https://example.com/manifest/1"
  options={{
    customLoadingComponent: () => <>My custom loading component</>,
  }}
/>

@joseph-bayer
Copy link
Contributor Author

Hey @joseph-bayer I'm going to review this as-is but perhaps we can still work towards updating the default with a skeleton? I image something very simple that grows and conforms with the general layout of Clover. The only large rectangle would be the canvas where images/video are rendered, something giving the impression of rich content.

██████████████

███████████████████████   █████████
███████████████████████   █████  
███████████████████████   ███████   
███████████████████████   ████████ 
███████████████████████   
███████████████████████

Related but not blocking changes is getting Clover fully working with Radix UI Themes which has a Skeleton component https://www.radix-ui.com/themes/docs/components/skeleton. This work is being completed in #202 but is a bit stale right now.

I think this is totally doable. We could even add/remove certain elements of the skeleton based on the config (ex: if the information panel is closed by default).

I you'd like, I could implement a solution independent from Radix (just pure html + css), and leave a TODO: comment in the code to utilize the Radix Skeleton component once themes are implemented.

Should I create a separate issue + pull request for this?

Copy link
Member

@mathewjordan mathewjordan left a comment

Choose a reason for hiding this comment

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

🌟

@mathewjordan mathewjordan merged commit 386660a into samvera-labs:main Feb 6, 2025
1 check passed
@mathewjordan
Copy link
Member

Hey @joseph-bayer I'm going to review this as-is but perhaps we can still work towards updating the default with a skeleton? I image something very simple that grows and conforms with the general layout of Clover. The only large rectangle would be the canvas where images/video are rendered, something giving the impression of rich content.

██████████████

███████████████████████   █████████
███████████████████████   █████  
███████████████████████   ███████   
███████████████████████   ████████ 
███████████████████████   
███████████████████████

Related but not blocking changes is getting Clover fully working with Radix UI Themes which has a Skeleton component https://www.radix-ui.com/themes/docs/components/skeleton. This work is being completed in #202 but is a bit stale right now.

I think this is totally doable. We could even add/remove certain elements of the skeleton based on the config (ex: if the information panel is closed by default).

I you'd like, I could implement a solution independent from Radix (just pure html + css), and leave a TODO: comment in the code to utilize the Radix Skeleton component once themes are implemented.

Should I create a separate issue + pull request for this?

Yes please. New issue is fine as well. But a seperate PR makes sense. I'll send this through. Thanks again! Best.

@mathewjordan
Copy link
Member

This work has been versioned as @samvera/[email protected]

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.

2 participants