Polish preview accessibility and UX - #97
Conversation
dc648c7 to
17c7d64
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17c7d649b4
ℹ️ 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".
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a05075b3c
ℹ️ 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".
| const focusable = Array.from(modal.querySelectorAll<HTMLElement>( | ||
| 'button:not([disabled]), input:not([disabled]), select:not([disabled]), [tabindex]:not([tabindex="-1"])', | ||
| )).filter(el => !el.hidden && el.offsetParent !== null); |
There was a problem hiding this comment.
When any tab other than Models is active, the inactive Models button has tabIndex = -1 but still matches button:not([disabled]), so it is incorrectly treated as the first focusable element. Consequently, Shift+Tab from the actual active tab does not trigger the wrap condition and focus can escape the aria-modal asset browser; filter candidates by their effective tabIndex or explicitly exclude [tabindex="-1"] from every selector branch.
Useful? React with 👍 / 👎.
| row.setAttribute('role', 'button'); | ||
| row.tabIndex = 0; |
There was a problem hiding this comment.
For every file entry, assigning role="button" to the row creates an interactive button that contains the native Open button added later. This produces conflicting nested button semantics: assistive technologies can flatten button descendants, making the distinct Open action undiscoverable, while the outer control itself performs different actions for Space and Enter. Represent the selectable row with an appropriate non-button item role or place the Open button outside that interactive control.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40819da1d5
ℹ️ 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".
| </div> | ||
| <canvas id="gizmo" class="gizmo" width="80" height="80"></canvas> | ||
| <div id="loadingOverlay" class="wv-loading-overlay visible"> | ||
| <div id="loadingOverlay" class="wv-loading-overlay visible" role="status" aria-live="polite" aria-busy="true"> |
There was a problem hiding this comment.
When an image or model finishes loading, setLoading(false) only removes .visible and sets aria-busy="false"; the shared CSS hides the overlay using opacity alone, so this new role="status" remains in the accessibility tree with stale text such as “Loading image...”. Screen-reader users can therefore continue to encounter a loading status after the preview is ready; toggle aria-hidden or otherwise remove the completed overlay from the accessibility tree.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
What changed
Why
This completes the UX/accessibility polish pass for the MPQ viewer, object editor asset browser, model preview, and shared preview states.
Validation
npm testnpx tsc -p . --noEmitnpm run lint(existing unrelated webpack warning only)npm run compile-webgit diff --check