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 loading screen for crystal visualization #409

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

franflame
Copy link

Meant to implement a loading screen for the crystal visualization tool in place of the blank screen.

Using Loading() with no further changes results in a loading screen that is preceded by a blank white screen for longer loading times.

@mkhorton
Copy link
Member

mkhorton commented Nov 6, 2024

Hi @franflame, can you clarify? Are you saying the change you are suggesting does not work as you intend?

If I recall, the issue with putting the structure viewer within a Loading component is that it will flash the loading spinner each time a setting is changed, which also temporarily hides the controls, leading to a confusing user experience. However, I have not tried this recently!

Thank you for opening the PR.

@franflame
Copy link
Author

Hi @mkhorton,

Thanks for reaching out! Yes, unfortunately the loading screen does not fully work as intended. I don't recall encountering the issue you described, but I haven't looked into it specifically, so I can't say for sure whether it's still happening.

The current issue is a little different - instead of the spinner always being present when the screen is in a loading state, the screen will sometimes stay blank for a few seconds before the spinner is displayed. This tends to happen for visualizations with longer loading times. I've tried many permutations of where to place Loading(), but unfortunately the issue still persists.

Let me know if this helps, and if I can provide any further clarification.

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