8.2 KiB
Independent review: custom overlay palette follow-up
Reviewed b1e8e454cda3133622be8f96458514c975785a71 against base afa74a305980f611437dd3aa2f937d36e9266cfe, branch codex/palette-overlay-956atf-20260913, worktree .local/palette-overlay-20260913. Read the complete three-file diff, surrounding custom draw/update/layout paths and the author's handoff. The reviewer did not author this overlay contribution. Root owns integration, compiler execution and native acceptance.
No actionable source defect found. The contribution is suitable for the next identified integrated gate and native trial. This is source-review clearance, not a claim that the two new tests pass or that the monitor requirement is complete.
Production behavior and boundaries
The chooser's extra bottom rule is inside the existing highlighted().id == candidate.id paint branch. Enter obtains the same highlighted candidate and publishes the existing Choose action. Neither candidate sorting, query generation, Current reference, Write context nor event forwarding changes. Arrow navigation remains a Browse action; the rule is not painted on every row or on the separate current-reference label. The added x+2/right-2, bottom-3/bottom-1 quad stays within the 34-unit row and the existing popup clip. No label rectangles, hit regions, row counts, pagination, measurement or ellipsis change.
The context menu's inset outline likewise stays inside focused == Some(index) && message.is_some(). Its state reconciliation and movement skip nonactions/disabled entries; Enter reads that same focused enabled action. The original fill remains behind a transparent-fill border quad. The 1-unit inset and 1.5-unit border do not alter the measured row, label/shortcut margins, scrolling or pointer target. Degenerate dimensions skip only the new paint. Disabled labels keep their existing alpha and cannot receive the new target outline merely because a pointer crosses them.
The overflow thumb changes only its fill from LINE to BOUNDARY. Its track/size, drag mapping, scrolling state, hit exclusion and clip remain unchanged. The chooser popup boundary and ordinary enabled reference-trigger boundary likewise change only LINE to BOUNDARY. Trigger hover/focus colors and 1/2-unit widths remain; the existing on_open.is_some() condition still controls its interactive outline/focus/event behavior. No disabled trigger is made actionable.
The shadowed outer context-menu border and decorative menu separators remain LINE in this contribution. Consequently this review does not claim every popup outer outline has been strengthened. The author's handoff records that distinction explicitly; include it when assessing the actual popup silhouette in native witnesses.
Numerical checks
Executed dependency-free sRGB arithmetic using the previously reviewed calculator, preserving source floats for old LINE. Current opaque source pairs are:
| Pair | Contrast |
|---|---|
TEAL #00665C / white |
6.873775 |
TEAL / actual highlighted fill #DAEEEB |
5.698861 |
BOUNDARY #667586 / white |
4.715161 |
| Old LINE source floats / white | 1.298757 |
Thus the new cue has substantial source contrast against both the retained highlighted fill and ordinary white, and the changed boundaries/thumb improve the intended weak pair. These are specified opaque colors, not measurements of antialiased strokes, the unchanged shadow, disabled alpha, monitor calibration or physical gamut. The marker adds a geometric distinction that remains meaningful when hue is lost.
Evaluator review
context_menu::tests::mounted_keyboard_menu_paints_target_and_scroll_thumb_then_invokes_that_action uses the actual custom Popup, real UserInterface and supported WGPU Headless renderer. Its independent measurement of the existing MenuOverlay geometry is used only to locate pixels; it does not replace the rendered overlay. The fixture includes headers, separators, disabled actions and overflow. Initial target index 1 and post-ArrowDown target index 4 correspond to the existing actions, while indices 2 and 5 are disabled. No mouse cursor supplies accidental hover. Target movement must emit no action; two Enters must emit exactly Properties action 22 once. The actual thumb center is sampled before hover.
forms::reference_chooser::widget::paint_tests::mounted_chooser_keyboard_target_is_visible_and_enter_keeps_exact_identity uses the actual chooser Popup and its search input. Reversed ingestion order prevents a lucky assumption that input order equals sorted candidate order. It preserves UI cache around the real controlled-state update: initial Alpha, ArrowDown to Beta ID12 with Outcome::Browsed and unchanged Current, then one exact scoped Write to ID12 despite two Enter events. Neither focus nor highlighted state is injected privately.
Both tests require a dark, nearly continuous lower-row band in over 80% of interior columns, and below 20% on previous/ordinary rows. The chooser includes the descender-containing context System / supply topology. That, together with the explicit inactive-row checks, makes scattered label pixels an implausible substitute for the new full-width cue. The old pale fill cannot pass this criterion. The test probes are inside fixture bounds and avoid row corners and the scrollbar. UserInterface::draw resets rendering between frames, so old target pixels do not accumulate into the next witness.
These are appropriate regression evaluators for the custom overlay path, rather than tests of a stock Iced pick-list or copied theme object. They retain exact action/identity checks as well as paint checks. Existing stale-selection, disabled-row, clipping, paging, scroll and release tests are unchanged.
The pixel criterion is contrast against white; the actual selected-row adjacent fill is pale green. The additional source calculation above checks that current cue against that fill too. A future arbitrary palette change should not treat the white-reference pixel assertion as an exhaustive adjacent-surface contrast contract.
The new tests do not assert painted trigger/popup-border continuity, although the one-token changes there are straightforward and their source contrasts were checked. The native blank-trigger/popup-boundary witness remains necessary. Their source simplicity does not justify a claim of calibrated display acceptance.
Remaining acceptance limits
The chooser rule sits inside the lower part of the second line's 17-unit label rectangle, close to the ordinary 10 px glyph extent. The source comment's intent to place it below the two lines is reasonable for the current layout, but not a proven font/DPI invariant. Text remains painted with its original bounds and metrics; the new tests establish a visible rule, not that every descender stays visually separate from it. Inspect the actual descenders, ellipsized contexts and narrow supported window at the target DPI before accepting the appearance. This risk is already named in the author's handoff and does not require a general chooser refactor in this bounded source contribution.
Neither new test was executed by this reviewer, and both were uncompiled/unrun in the author's source handoff. Compiler/type compatibility, actual WGPU assertions, keyboard-only native chooser/menu tasks, ordinary blank-trigger visibility, menu/chooser outer silhouette and scroll-thumb appearance remain execution/native gates. Search/warning bridges, additional chooser semantics and unrelated control work are outside this review; none is made a new prerequisite here.
Identity and performed checks
The worktree was clean. Each reviewed local file was compared byte-for-byte to git show b1e8e454cda3133622be8f96458514c975785a71:<path> and matched. git diff --check afa74a3..b1e8e45 produced no findings. No compiler/Cargo/rustc, native UI, Gitea or production edit was performed by the reviewer.
| File | SHA-256 |
|---|---|
crates/oa-desktop/src/context_menu.rs |
4a4d8893660e6a550b127d31ff9ece3d4ec413cef0747b496be90949436c6523 |
crates/oa-desktop/src/forms/reference_chooser/widget.rs |
875a621e8a13bfc72cf430d6df416bde023930974d7e06728be96996efcc08fa |
crates/oa-desktop/src/forms/reference_chooser/trigger.rs |
5c180c18775186eae997814e9c0050813edb202c535cee665e046d40aadb52d7 |