Skip to content

Keep embedded shortcuts readable across button states - #97

Merged
alexschmander merged 1 commit into
feat/current-user-activityfrom
fix/shortcut-label-contrast
Sep 7, 2026
Merged

Keep embedded shortcuts readable across button states#97
alexschmander merged 1 commit into
feat/current-user-activityfrom
fix/shortcut-label-contrast

Conversation

@alexschmander

@alexschmander alexschmander commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Behavior or contract

Embedded keyboard shortcuts now use each button's foreground, so highlighted and selected buttons keep their shortcut text readable. Disabled semantic buttons preserve disabled semantics and a faded appearance with sufficient text contrast. Danger buttons retain their red fill/border and use the normal readable text foreground. Adjacent supporting text keeps its existing styling.

Refs #14.

Production ownership path

The shared theme::button_shortcut supplies egui's state-dependent foreground placeholder for every production button shortcut. The semantic helpers and selected keypoint placement buttons share a scoped renderer that restores style and opacity after rendering. keypoint_placement_mode preserves its action names and exposes the complete configured shortcut in its tooltip and accessible description. The native inspector and production WASM reach the same rendering owners. No bindings, domain/API/persistence contracts, or action allocation changed. The manual migration primary button now composes directly with the shared shortcut renderer after removal of the daily footer.

Acceptance criteria and evidence

The regression coverage below exercises this PR’s behavior. Focused regression checks and the native/browser evidence below cover the final behavior, with source revisions distinguished.

Regression protection

Before the implementation, the painted contrast test failed on primary hover/focus around 1.03:1, primary disabled around 1.34:1, and other selected/disabled/danger combinations. contrast-fail-before.log retains that executed failure. After the shared fix, all 120 cases pass. An expanded parent-disabled/main-label test initially caught an incorrect disabled foreground assumption; the final renderer now uses egui's actual inactive foreground and passes that case too.

Verification

Head 6fd38c225c02d8a7e7c0d85117b170a08cbe9c82, comparison base 5d23a471796ce3553ba995c750f135c3bc7466b8. This PR retains its own commit in stack #118, rooted at main 37c207f84c0aadf570b1409da4693b1bc070a280.

The navigation-order and centered-badge adjustment received minimal verification at the user’s explicit request. Formatting, the five presence tests, desktop action order and badge centering, drawer navigation, responsive header and long-status checks passed. Native inspection passed at 1440×1000 and 320×320 on the exact final header source. All verification used GPT-5.6-Luna at max reasoning.

Earlier complete canonical per-PR and combined checks passed on the presence implementation. Hosted Testing passed for all five alignment heads ending at ed96e02c5c95d903d467ddae23ae65d8f2472299. The final small header adjustment was not given another full canonical run; hosted CI has now passed on the current head.

Visual or browser evidence

Chromium 153 passed the presence failure/recovery and actual lease-release checks on ed96e02c5c95d903d467ddae23ae65d8f2472299, including three failed polls with zero mutation requests, no daily-stat polling, 13 viewport/DPR cases, keyboard/wheel/Fit zoom and actual 200% browser zoom. Native inspection covered annotation, review and migration at six viewports, all status colors, wrapped details and resize focus.

The final adjustment only changes navigation order and badge placement. A private native rebuild of that exact source confirms presence → dot → Logout → Setup → Admin → Settings → Statistics at 1440×1000, with the dataset badge centered. At 320×320 the presence, dot and navigation trigger remain within the viewport and the badge yields. Evidence is retained locally under /tmp/labello-presence/native-final and /tmp/labello-presence/browser; fixture changes were restored and private runtimes stopped.

Documentation

Current documentation remains aligned with this PR’s behavior and the revised presence header inherited from #100. No dependency or persistence-format changes.

Risks and omitted checks

Browser egui exposes Canvas rather than native widget semantics; native inspection and kittest cover widget labels and focus. Browser font preferences do not establish egui text-only scaling support. Physical-device behavior is not claimed. Lease expiry is unchanged, so closing a browser does not immediately remove that user. Per the user’s request for minimal verification, the final navigation-only adjustment did not repeat the full native/browser matrix or canonical suite.

Worktree integrity

  • Unrelated worktree changes were preserved and excluded.
  • No credentials, runtime datasets, generated distributions or lockfile changes are included.

Handoff status

  • Requested focused local verification complete.
  • Required Testing check passed on this exact head.
  • Existing draft state, assignments and reviewer requests preserved.

Independent acceptance and merge remain separate from this implementation update.

Latest correction reverses only the header icons: right to left, Logout, Setup, authorized Admin, shortcut settings, statistics. The focused desktop-header test and formatting check passed. Dataset, presence and drawer layout are unchanged. Earlier native captures retain their original icon-order provenance; hosted CI has now passed on the current head.

CI fix: the workflow test now opens Tutorial with its existing keyboard shortcut instead of looking for the removed header action. The corrected workflow test, all 438 UI tests with all features, doc-tests and formatting pass locally. Cargo test was used because local cargo-nextest is unavailable. Production UI is unchanged by this fix. Hosted Testing passed on this new head.

Current-head CI: all jobs, including UI and Testing, passed on 6fd38c225c02d8a7e7c0d85117b170a08cbe9c82. Successful run. Remote heads and chained bases match the stack, and the final source tree matches the locally verified test-fix tree.

@alexschmander alexschmander self-assigned this Sep 5, 2026
@alexschmander
alexschmander marked this pull request as ready for review September 5, 2026 16:47
@alexschmander
alexschmander marked this pull request as draft September 5, 2026 20:18
@alexschmander
alexschmander changed the base branch from main to fix/measured-workspace-overflow September 5, 2026 21:34
@alexschmander
alexschmander marked this pull request as ready for review September 5, 2026 21:38
@alexschmander
alexschmander marked this pull request as draft September 5, 2026 21:52
@alexschmander
alexschmander marked this pull request as ready for review September 5, 2026 22:40
@alexschmander
alexschmander marked this pull request as draft September 6, 2026 07:35
@alexschmander
alexschmander marked this pull request as ready for review September 6, 2026 07:37
@alexschmander
alexschmander changed the base branch from fix/measured-workspace-overflow to feat/current-user-activity September 6, 2026 07:37
@alexschmander
alexschmander force-pushed the fix/shortcut-label-contrast branch from cbd8ecf to d697811 Compare September 7, 2026 13:43
@alexschmander
alexschmander force-pushed the fix/shortcut-label-contrast branch from d697811 to b3d43ad Compare September 7, 2026 16:17
@alexschmander
alexschmander force-pushed the fix/shortcut-label-contrast branch 2 times, most recently from 6bede3f to 5af654f Compare September 7, 2026 16:49
@alexschmander
alexschmander force-pushed the fix/shortcut-label-contrast branch from 5af654f to 56bd4ff Compare September 7, 2026 16:53
@alexschmander
alexschmander force-pushed the fix/shortcut-label-contrast branch from 56bd4ff to 6fd38c2 Compare September 7, 2026 17:02
@alexschmander
alexschmander merged commit 4d9a04b into main Sep 7, 2026
10 of 20 checks passed
@alexschmander
alexschmander deleted the fix/shortcut-label-contrast branch September 7, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant