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

Feature/thumbnail loading placeholder #268

Merged
merged 6 commits into from
Oct 17, 2024

Conversation

BrianWhitneyAI
Copy link
Contributor

@BrianWhitneyAI BrianWhitneyAI commented Oct 11, 2024

Description

The purpose of this PR is to resolve #161 and add a loading state to thumbnails that we are rendering. In the course of this PR the following logic was added:

  • Loading state for thumbnail processes
  • Try logic / failed state for zarr thumbnails
  • Retry logic for Zarr thumbnails (3) + (Timeout)

SCREENSHOTS

Small icon loading

Screenshot 2024-10-11 at 12 11 01 PM

Failed Zarr (top left corner)

Screenshot 2024-10-11 at 12 10 43 PM

styles.fileThumbnail,
styles.thumbnailPlaceholder
)}
style={{ height: props.height, width: props.width }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this default to being a square if one of the fields is present but not the other? Like so:

style={{ height: props.height || props.width, width: props.width || props.height }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved in 478fbd2

@BrianWhitneyAI
Copy link
Contributor Author

Currently has a visual bug where the loading box / spinner is misaligned. I tried recentering but somethings weird.
Screenshot 2024-10-15 at 12 35 45 PM

@BrianWhitneyAI BrianWhitneyAI marked this pull request as ready for review October 16, 2024 21:43
let attempt = 0;
while (attempt < retries) {
try {
return await withTimeout(fn(), timeout);
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of the timeout here. Does it help the callstack avoid getting overloaded somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what i can tell some async requests in the zarr render dont register as failures, This just says if its taking too long count it as a failure.

@BrianWhitneyAI BrianWhitneyAI merged commit 8424535 into main Oct 17, 2024
3 checks passed
@BrianWhitneyAI BrianWhitneyAI deleted the feature/thumbnail-loading-placeholder branch October 17, 2024 17:04
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.

Loading Icon for rendered Zarr images
3 participants