[codex] Fix web WASM loader and dialogs#15
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new loading overlay and modal for the web application, enabling users to select .ld and .ldx files simultaneously. Key UI enhancements include the addition of a favicon, a "Close" button for the channel picker, and explicit repaint requests in the App struct to ensure UI responsiveness. Furthermore, specific background job functions have been conditionally compiled to exclude them from WASM targets. I have no feedback to provide as no review comments were present.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5eb954862d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sessionFilesInput.addEventListener("change", () => { | ||
| if (sessionFilesInput.files && sessionFilesInput.files.length > 0) { | ||
| void loadSelectedFiles(); | ||
| } |
There was a problem hiding this comment.
Reset file input to allow selecting the same session again
The new auto-load flow only runs on the file input change event, but the input value is never cleared after success or failure. In browsers, choosing the same file list again does not fire change, so after a failed parse/startup attempt users cannot retry the same .ld/.ldx pair from this overlay without first selecting a different file or refreshing.
Useful? React with 👍 / 👎.
| const ldxFile = ldxFiles[0]; | ||
| let ldxText = null; |
There was a problem hiding this comment.
Reject multiple .ldx files in the combined picker
The combined picker now accepts multiple files, but only .ld count is validated; .ldx uses ldxFiles[0] and silently ignores additional sidecars. If a user selects more than one .ldx, the app may attach the wrong sidecar without warning, leading to incorrect channel metadata for the loaded session.
Useful? React with 👍 / 👎.
Summary
.ldand optional.ldxfiles together with one multi-file picker.0.5.0for the next build on merge.Root Cause
The web loader called directly into the running egui app and updated app state, but that path did not request a repaint. The status DOM updated, while the canvas could remain on the stale pre-load frame. The Manage Channels dialog is shown as an egui viewport; on web it does not get a native window close control, so it needed an explicit in-app close affordance.
Validation
cargo check -q -p i3rs-appcargo check -q -p i3rs-app --target wasm32-unknown-unknowncargo fmt --checkenv -u NO_COLOR trunk build --releasedistserver for sample loading and modal behavior.