RESIZE KEYS K4 follow-up: a row's minimum rounds up, so no dead row key press (review notes A, B, C of #32471)

Merged
LA
5d ago
+154-8

Summary

Follow-ups of the RESIZE KEYS K4 review (#32471, re-review SHIP at 76e9d242628, comment 6585271874, notes A, B, C):

  • C (code, final form after review): a row's minimum height in permille came from RowStackGeometry.permille(forPixelHeight:), which rounds to the nearest permille, so it could be up to half a permille below the real minimum. A row stored one permille above that floor took an Up press (Ctrl-Shift-K / Ctrl-Cmd-Up) that changed the model and moved no pixel: a dead press. ShownResizeRange.rowHeights now takes the largest stored permille that still shows at the minimum on the screen's pixel grid (the backing scale comes from LayoutRootView), and each press direction checks only its own bound.
  • A (test): a scrolling stack [520, 500] pressed Up stops exactly at [500, 500] (the #32471 test could not fail for this).
  • B (test): the middle of three rows moves its lower edge; the top row stays.

Testing

On the fleet (scripts/ci/package-test-lane.sh suite Packages/macOS/CmuxNext ...), compile-checked first. Same three commits as on the earlier branch (fix/resize-keys-rows-followup-2), cherry-picked onto feat after #32471 landed:

  • First red ecf6522a498: step e7a1ebcb854157ef99fd6a3e PASSED: no red (its few pane stacks never hit a minimum just under a half permille). The test was rewritten.
  • Red 3f9caa0d240 (test C sweeps the minimum content height 64...400 pt by 0.25): step 0238a2a684d3c6729caf3137 FAILED as expected: 54 dead presses. A and B pass on the old code (they pin #32471's behavior).
  • Fix ff9dbbea052: green + GUI proof (display worker gui-session-v1, CMUX_TEST_REQUIRE_GUI=1) step ec668ceba17c41f19174e264 PASSED: KeyboardResizeRowsFollowupTests 3/3, KeyboardResizeRowsEdgeCasesTests 5/5, KeyboardResizeRowsTests 2/2, KeyboardResizeShownLimitTests 2/2.
  • This head 77abc43df418d3206e0de9fb02400df1b0e9d460: compile-check runs; GUI proof at this exact head runs before ready-to-land.

Review FIX FIRST at 77abc43df41 (comment 6587348634): the floor rounded up from the exact placement could show one device pixel above the minimum, and dead presses remained at 1x. Then:

  • Red aead1169c6f (KeyboardResizeRowsPixelFloorTests at 1x and 2x; ShownResizeRange takes the backing scale): step 1165ac076f27889b2fc1771f (aws-m4pro-4) FAILED as expected (dead presses at 1x, rows stopping above the minimum at 1x and 2x).
  • Fix 15df074b5ac (floor = the largest stored permille that still shows at the minimum on the pixel grid): green + GUI step 7d3cdbc426944307db777996 (cmux14) FAILED: a real fault: Up was refused 0.5-2.5 pt above a tall row minimum where the two rows' minimums do not both fit (stepped() refused every press when low > high, but high bounds Down only).
  • Fix 429fea28b2a (each direction checks only its own bound): compile-check d1c80cab9116595a31aa0175 PASSED; green + GUI proof (display worker gui-session-v1, CMUX_TEST_REQUIRE_GUI=1) at the exact head 429fea28b2adf91a92512d9bb519b16eda1eb69b: step 307a58a0d37a11f3b30f3c21 on cmux-austin-mini-0 PASSED, 18 test cases, none skipped (these suites need no daemon): PixelFloor (1x, 2x), Followup 3/3, RowsEdgeCases 5/5, Rows 2/2, ShownLimit 2/2. Five identical submissions coalesced into this one step; the suites have no timing, so a repeat adds no evidence.

Observed / Not run

  • Observed: the steps above.
  • Not run: a film; the dead press needs a row stored exactly one permille above its minimum, which a person reaches only by a drag in narrow cases.

No capture: geometry rounding only.

Changelog

  • Fixed: Ctrl-Shift-K / Ctrl-Cmd-Up next to a row's minimum height no longer takes a key press that moves nothing.

Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.

Summary by cubic

Fixes the dead row-resize key press next to a row's minimum so Ctrl-Shift-K / Ctrl-Cmd-Up always moves an edge or is refused at 1x and 2x.

ShownResizeRange now takes the screen's backing scale and derives the floor as the largest stored permille that still shows at the row's minimum on the pixel grid, replaying RowStackGeometry's placement. This replaces both the nearest-permille rounding (up to half a permille too low, causing dead presses) and the exact-placement rounding up (which could show one pixel above the minimum and still dead-press at 1x). Up presses now stop only at the upper row's minimum, so Up still moves the edge where both rows' minimums cannot fit — the lower row only grows.

  • Adds tests for review notes A, B, C of #32471: a scrolling stack stops exactly where it fills the column, a middle row moves only its lower edge, and a sweep of minimum content heights catches previously dead presses.
  • Adds a 1x/2x pixel-floor test: from any stored height just above the minimum, Up presses either move the edge or are refused and end with the row at its minimum.