Repository navigation
feat(ui): float input suggestions in the work area's bottom-right corner - #56
Merged
Merged
Conversation
`ui/src/placement.rs`, `ui/src/workarea.rs`, `ui/src/suggestion.rs` and `ui/suggestion.slint` were written but never declared in `ui/src/main.rs`, so none of it was compiled into the binary. A suggestion was always drawn on the resident panel's confirmation card, and nothing measured otherwise. Declaring the modules makes the feature reachable. What the working version needed beyond that is the four things below. **Placement against the work area, not the monitor.** A card inset from the screen edge sits under the taskbar: visible in a screenshot, unreachable in life. `workarea.rs` asks Win32 for `rcWork` through two hand-declared imports rather than taking `windows-sys` as a direct dependency. `placement.rs` keeps every coordinate conversion in one function — the first version scaled the card but not its inset, which put the card a few pixels off the corner at any DPI above 100%, and the smoke caught it by disagreeing with a panel that had already read the real work area. **The card does not take the keyboard.** Slint 1.18 exposes `always-on-top` and `no-frame` but no activation hint, and re-showing a window on Windows sets `SW_SHOW` on every show after the first (`window_state.rs` latches on `MARKER_ACTIVATE`). `keyboard.rs` adds `WS_EX_NOACTIVATE` and restores the previous foreground window after each show. The harness now measures this instead of trusting the comment: `ui:smoke` records who holds the keyboard before and after the card appears. **Closing the card sets it aside; it does not answer.** A frameless card has no close button, but Alt+F4 still asks. Routed through the same path as `Later`, so the panel keeps a notice offering to bring the prompt back rather than leaving it believing the card is still on screen. The same rule governs closing the panel: it is "put away", and the engine's own deadline is the only thing that decides an unanswered prompt. **Where there is no corner, the panel asks.** `workarea.rs` returns `None` off Windows — the whole screen would be a guess that no desktop has panels, true on a bare Xvfb and false on every real Linux desktop, where the card would open under the dash. A screen too small for the 360px card returns `None` too. Both land the prompt on the panel's card rather than nowhere, and the smoke asserts that branch rather than skipping it. The smoke had never sent a suggestion: it only ever sent `kind: "run_tests"`. Adding the case required a streaming `push` on the fixture and three host capabilities — `findWindow` (answers "must not be there" without waiting), `workArea`, and an optional `foregroundWindowTitle` that a host may decline. The corner assertion deliberately checks containment and nearness rather than one exact coordinate: the first version restated the panel's `CORNER_INSET` as a bare 16 physical pixels while the panel correctly scales it, and a harness that called correct placement wrong would have taught the next person to loosen it. Exact arithmetic stays in `placement.rs`, where it tests without a display. Also fixes a flaky assertion in `tests/screen-text-reader-streaming.test.ts`: it asserted an exact child count after a fixed 30ms sleep, but the loop correctly restarts a child that stalls again, so both 1 and 7 are reachable and neither is a property of the loop. It now polls for the condition and asserts "killed, said why, replaced". Verified: `cargo clippy --all-targets` clean, `cargo test` 104 passed, `bun test` 826 passed / 23 skipped / 0 failed, `bun run typecheck` clean, and `bun run ui:smoke` green on a 2560x1380 work area at 125% — the card lands at its corner and the keyboard stays with the window that had it.
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. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
`answer`, `defer` and `cancel` each hid the floating card from inside either an `if let Some(panel) = panel.upgrade()` or a `panel.upgrade_in_event_loop` closure, which made the card's removal conditional on the panel still being alive. They are separate top-level windows and it is not. The case they diverge is the one the tray's Quit callback walks: it answers the pending prompt and then ends the event loop, so the answer is written and the card is left on screen holding buttons for an id that has already been spent. A click on it finds an empty slot and returns, which is precisely the dead surface `answer`'s take-the-id-out exists to prevent. All three now hide the surface first, on the UI thread they are already on, and only then go looking for the panel to update. `cancel` also passes `clear_prompt` directly rather than wrapping it in a closure that no longer captures anything. Found by reading the callbacks back after the smoke went green — the harness does not cover a panel that closes mid-prompt, and the tray quit path is the one place in `main.rs` that answers and then stops. Verified: `cargo clippy --all-targets` clean, `cargo test` 104 passed, `bun test` 826 passed / 23 skipped / 0 failed, `bun run ui:smoke` green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(ui): float input suggestions in the work area's bottom-right corner
ui/src/placement.rs,ui/src/workarea.rs,ui/src/suggestion.rsandui/suggestion.slintwere written but never declared inui/src/main.rs, sonone of it was compiled into the binary. A suggestion was always drawn on the
resident panel's confirmation card, and nothing measured otherwise.
Declaring the modules makes the feature reachable. What the working version
needed beyond that is the four things below.
Placement against the work area, not the monitor. A card inset from the
screen edge sits under the taskbar: visible in a screenshot, unreachable in
life.
workarea.rsasks Win32 forrcWorkthrough two hand-declared importsrather than taking
windows-sysas a direct dependency.placement.rskeepsevery coordinate conversion in one function — the first version scaled the
card but not its inset, which put the card a few pixels off the corner at any
DPI above 100%, and the smoke caught it by disagreeing with a panel that had
already read the real work area.
The card does not take the keyboard. Slint 1.18 exposes
always-on-topandno-framebut no activation hint, and re-showing a window on Windows setsSW_SHOWon every show after the first (window_state.rslatches onMARKER_ACTIVATE).keyboard.rsaddsWS_EX_NOACTIVATEand restores theprevious foreground window after each show. The harness now measures this
instead of trusting the comment:
ui:smokerecords who holds the keyboardbefore and after the card appears.
Closing the card sets it aside; it does not answer. A frameless card has no
close button, but Alt+F4 still asks. Routed through the same path as
Later,so the panel keeps a notice offering to bring the prompt back rather than
leaving it believing the card is still on screen. The same rule governs closing
the panel: it is "put away", and the engine's own deadline is the only thing
that decides an unanswered prompt.
Where there is no corner, the panel asks.
workarea.rsreturnsNoneoffWindows — the whole screen would be a guess that no desktop has panels, true on
a bare Xvfb and false on every real Linux desktop, where the card would open
under the dash. A screen too small for the 360px card returns
Nonetoo. Bothland the prompt on the panel's card rather than nowhere, and the smoke asserts
that branch rather than skipping it.
The smoke had never sent a suggestion: it only ever sent
kind: "run_tests".Adding the case required a streaming
pushon the fixture and three hostcapabilities —
findWindow(answers "must not be there" without waiting),workArea, and an optionalforegroundWindowTitlethat a host may decline.The corner assertion deliberately checks containment and nearness rather than
one exact coordinate: the first version restated the panel's
CORNER_INSETasa bare 16 physical pixels while the panel correctly scales it, and a harness
that called correct placement wrong would have taught the next person to loosen
it. Exact arithmetic stays in
placement.rs, where it tests without a display.Also fixes a flaky assertion in
tests/screen-text-reader-streaming.test.ts:it asserted an exact child count after a fixed 30ms sleep, but the loop
correctly restarts a child that stalls again, so both 1 and 7 are reachable and
neither is a property of the loop. It now polls for the condition and asserts
"killed, said why, replaced".
Verified:
cargo clippy --all-targetsclean,cargo test104 passed,bun test826 passed / 23 skipped / 0 failed,bun run typecheckclean, andbun run ui:smokegreen on a 2560x1380 work area at 125% — the card lands atits corner and the keyboard stays with the window that had it.