Repository navigation
Tests: the Notes-button test clicks where the button is laid out, so it passes without Inter - #1300
Conversation
…it passes without Inter status::tests::a_long_language_name_leaves_the_notes_button_clickable clicked x 150 for the gap before the status bar's controls and x 200 for the Notes button. Those positions hold for Inter. With a craft-fonts checkout that has no fonts/latin-manifest.txt, the interface text is set in BIZ UDPGothic. Labels set in BIZ UDPGothic are wider than in Inter, so the Notes button starts at x 149. The first click then toggled the notes pane and the second toggled it back. The status bar's Notes and Comments buttons now report widget info with their names. The test reads the Notes button's rect, clicks midway between x 132 and its left edge, and then clicks its center.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Button metadata and click test crates/ui-egui/src/status.rs |
labeled reports the button label and selected state. The test queries the Notes button bounds and checks that a click in the gap does not open the Language dialog or toggle Notes, while a click on Notes toggles the pane. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Other
Possibly related PRs
- storytold/deckcraft#1260: Introduced the status-bar layout change and fixed-coordinate Notes click test that this PR updates.
Suggested reviewers: aiongg
Merge Risk: 
c207d
-
No actionable merge-blocking risk remains.
Architecture risk: 
c207d
-
The change affects 1 system.
Changed systems:
cratesArchitecture concerns
No architecture-level concerns identified.Review details
-
Systems and components
- observed — crates (service) was modified; 1 changed file maps to changed impact.
Before / after behavior
- observed — Modified behavior in crates/ui-egui/src/status.rs:
labelednow reports button widget information with its label and current selected state. - observed — Modified behavior in crates/ui-egui/src/status.rs: The test module now imports
Queryablefor querying rendered UI elements. - observed — Modified behavior in crates/ui-egui/src/status.rs: The test replaces hard-coded control positions with the computed right-controls boundary and the laid-out Notes button rectangle, and begins a click helper that accepts a position.
- observed — Modified behavior in crates/ui-egui/src/status.rs: The test clicks the gap between the control boundary and Notes, asserting that neither the Language dialog nor Notes state changes, then clicks Notes by its rendered center. This replaces clicks at fixed x-coordinates.
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title names the test change: the Notes-button test now clicks where the button is laid out, so it passes without the Inter font. The changeset matches this. The status.rs test replaces fixed click… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title names the test change: the Notes-button test now clicks where the button is laid out, so it passes without the Inter font. The changeset matches this. The status.rs test replaces fixed click coordinates with positions derived from the Notes button's reported widget rect, and the labeled widget metadata supports the lookup. The title is specific, but it is long at 92 characters and it omits the accessibility label change, so it is not a perfect summary. It is still related to the main change and is not misleading.
-
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
On Windows 11 on
mainat38d2ade,status::tests::a_long_language_name_leaves_the_notes_button_clickablefails with "the click reached the Notes button" whenCRAFT_FONTS_DIRpoints at a craft-fonts checkout of itsmainbranch.The test clicked x 150 for the gap before the status bar's controls and x 200 for the Notes button. Those positions hold for Inter. craft-fonts
mainhas nofonts/latin-manifest.txt, so Inter is not embedded andtheme::font_definitionssets the interface text in BIZ UDPGothic. Labels set in BIZ UDPGothic are wider than in Inter, so the Notes button is laid out farther left. Measured at 700 x 800 on Windows 11:main4391305, which CI uses)The language name's click area ends at x 132 in all three cases, so the status bar is laid out correctly and the test's fixed positions are the fault.
What changes:
nav_iteminbackstage.rsdoes, so screen readers and tests can find them.get_by_label("Notes"), checks that it starts after x 132, clicks midway between x 132 and the button, and then clicks the button's center. It checks the same things as before, and also that the click in the gap leaves the notes pane closed.Test: with the language name's clip widened to the bar's right edge, so that its click area covers the controls, the test fails with "the Language dialog stayed closed". With the change it passes with craft-fonts
main, with Inter only and with no craft-fonts.Checks:
cargo fmt --all -- --check,cargo clippy -p deckcraft-ui-egui --all-targets -- -D warnings,cargo xtask assetsandcargo xtask layers.On Windows 11 (x86-64) with rustc 1.99.1, on top of
mainat38d2ade, with #1292 merged in:cargo xtask cipasses its 8 steps: 1,877 tests passed, 0 failed, 6 ignored.git merge-treereports no conflicts withmainat38d2ade.Summary by CodeRabbit
Accessibility
Bug Fixes