-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/loading overlay #162
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
base: main
Are you sure you want to change the base?
Conversation
…ellpack-client into fix/vertical-spacing
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…ellpack-client into fix/vertical-spacing
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a loading overlay to the viewer to display the current state of the packing job. The overlay shows different messages based on whether data is loading, packing is running, or results have been modified since the last run.
Key changes:
- Added
editsfield toPackingResultto track recipe state at packing time - Created derived selectors
useIsLoadinganduseIsModifiedto determine overlay state - Implemented viewer overlay with conditional rendering and theme-aware styling
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/index.ts | Added edits field to PackingResult type |
| src/state/constants.ts | Updated EMPTY_PACKING_RESULT with empty edits object |
| src/state/store.ts | Added useIsLoading and useIsModified selector hooks |
| src/style/themeRoot.tsx | Wrapped app in theme-specific div for CSS targeting |
| src/components/Viewer/style.css | Added styling for overlay and theme-specific colors |
| src/components/Viewer/index.tsx | Implemented overlay with conditional rendering logic |
| src/components/PackingInput/index.tsx | Refactored to use useIsLoading hook |
| src/App.tsx | Updated to capture and store edits in packing results |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ellpack-client into feature/loading-overlay
Co-authored-by: Copilot <[email protected]>
|
Can you re-merge main on this? It seems to have a lot of changes in the changeset that have already been merged |
…nto feature/loading-overlay
|
On the preview link, it seems like the "Running..." overlay is removed slightly too early. When I trigger a re-run, the "Running..." overlay shows up as expected, but when the packing is complete and the overlay is removed, I see the previous result for a second, then it kinda flashes, then the new result shows up. Not sure what the best way to handle this is, but maybe we could at least clear out the old result URL in the simularium viewer while the job is running so if we show the simularium viewer early, it's a blank viewer? Or somehow make sure the Viewer has been updated with the correct result URL before we remove the overlay? |
Problem
Closes #136 and #137
Made a new branch because merge conflicts were bumming me out.
Solution
Conditionally render overlay, spinner and text based on state of the app.
Notable changes:
useIsLoadinganduseIsModifiededitsfield toPackingResulttype to store a snapshot of edits related to that packing job. Used for comparison to recipe edits inisModified.themeRootI wrapped the app in a div withdark-themeandlight-themeclass names so we can access the theme in downstream components that are not handled fully by theantdtheme provider.Screenshots show some small spacing issues that were solved by pointing this branch at
fix/vertical-spacingIn terms of viewer overlay I think the most important thing is that the overlay and the viewer are the same size.

