Repository navigation
fix(zones): expose source errors to assistive tools - #408
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: ffc25337cd
ℹ️ 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".
There was a problem hiding this comment.
🟢 Approval recommended
The changes are narrowly scoped to accessibility metadata on existing UI elements and are backed by targeted QML regressions that exercise dynamic updates and clearing behavior.
Pull request overview
This PR fixes an accessibility gap in the Zones “Sources” UI by ensuring existing source-mutation error messages are represented in the native accessibility tree (static-text role + name bound to the current error text), and adds focused QML regressions to prevent regressions in role/name updates, visibility, and editor/section behavior.
Changes:
- Add
Accessible.StaticTextrole andAccessible.namebindings to the two existing source-mutation errorTextelements (editor + section). - Add QML component tests asserting the error text becomes accessible, updates dynamically, and clears when the error is empty.
- Add objectNames for the two error
Textitems to enable stable test lookups.
File summaries
| File | Description |
|---|---|
| tests/qml/tst_zones_components.qml | Adds regression tests verifying accessible role/name behavior for source-mutation error feedback in editor and section contexts. |
| qml/features/zones/controls/ChannelSourcesSection.qml | Exposes section-level source-mutation error text to accessibility tools via explicit role/name and adds an objectName for testing. |
| qml/features/zones/controls/ChannelSourceEditor.qml | Exposes editor-level source-mutation error text to accessibility tools via explicit role/name and adds an objectName for testing. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
23a4b6e to
39ed96f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39ed96fcd1
ℹ️ 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".
| Accessible.ignored: !visible | ||
| visible: root.zoneState.sourceMutationError.length > 0 | ||
| && (!root.conflict | ||
| || root.zoneState.sourceMutationError.toLowerCase() |
There was a problem hiding this comment.
When saving fails with a revision conflict, submit() sets root.conflict while the mutation error contains “revision conflict,” so this newly accessible node becomes hidden and ignored. The remaining visible explanation, “Source revision changed. Draft was not rebased,” is another plain Text item without an accessible role or name—the same configuration this change found was omitted from the accessibility tree—leaving assistive-tool users with a Reload button but no explanation. Give that replacement message an explicit Accessible.StaticText role/name and exclude it when its conflict row is hidden.
Useful? React with 👍 / 👎.
f140431 to
376c0a4
Compare
Give existing source mutation errors an explicit static-text role and bind their accessible name to the current feedback. Cover editor and section feedback without changing validation, persistence, or draft state. Verification: cargo fmt --all; cargo fmt --all -- --check; python3 scripts/check-build-pipeline.py ci (passed, including 1,344 QML tests); focused component suite (58 passed); native standalone and Basecamp rejection and valid-source E2E; standalone source persistence after restart. Native E2E used candidate QML with the previously verified native artifacts from #406. Fixes #407
Bind error accessibility exclusion to effective visibility so the hidden section message does not duplicate editor feedback and suppressed revision-conflict text is not exposed. Extend the component tests for conflict suppression, other errors during conflict, editor open and close, and empty feedback. Native standalone baseline exposes two identical error nodes; the corrected standalone and Basecamp hierarchies expose one. Verification: cargo fmt --all; cargo fmt --all -- --check; python3 scripts/check-build-pipeline.py ci (passed, including 1,344 QML tests); focused component suite (58 passed); native standalone and Basecamp rejection/hierarchy checks with unchanged source configuration; explicit Basecamp Stop. Native checks use candidate QML with previously verified #406 artifacts. Refs #407
376c0a4 to
a0f3124
Compare
Summary
Fixes #407.
Accessible.StaticTextrole and an accessible name bound to the current message.Accessible.ignored: !visibleso section/editor messages are not exposed twice.Timestamped native probing corrected the initial diagnosis: the error was rendered, but omitted from the accessibility tree. The observed later state reset happened during window closure, not an unrelated refresh. No state-retention workaround is included.
Verification
cargo fmt --allandcargo fmt --all -- --check: passed.python3 scripts/check-build-pipeline.py ci: passed, including workspace Clippy/tests, wallet isolation, native CTest, web checks, headless CLI and 1,344 QML tests.Native E2E isolated this QML-only change by loading candidate QML with previously verified native artifacts from #406 and blockchain #81. These are not whole-application binaries rebuilt from this branch. Temporary Basecamp QML staging was restored after testing. Screenshots were inspected; this patch has no visual layout or styling change.
The known caught-up catalog warning (#396) and historical provider ancestry issue (#274) remain separate.
Review follow-up
Commit
23a4b6e33f86e3d93e3eeb0fe1d7cdc7589bd226adds visibility-bound exclusion and extends tests for conflict suppression, non-conflict errors during conflict, editor open/close, and empty feedback. Both new hidden-state assertions fail on the preceding commit.Full local gate and all 58 component tests passed again on this head. Native standalone baseline exposed two identical errors in the hierarchy including hidden nodes; patched standalone and Basecamp each expose exactly one, preserve the visible rejection, and leave configuration unchanged. Basecamp explicit Stop passed. Temporary staging was restored.
The 70-second retention and valid-source/restart controls above were run on the preceding commit
ffc25337; the follow-up reran the exact rejection/hierarchy cases with the same native artifact provenance.