Skip to content

cmux NEXT Home: search text stays beside the magnifier while editing - #28021

Merged
austinywang merged 6 commits into
feat-cmux-nextfrom
27967-chief-search-align
Oct 30, 2026
Merged

austinywang merged 6 commits into
feat-cmux-nextfrom
27967-chief-search-align

Conversation

@austinywang

@austinywang austinywang commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #27967

Changelog: Fixed: the Home search field's text no longer slides over the magnifier while you type; it stays centred beside the magnifier, level with the filter button.

Cause

While editing, the Home list's unbezeled search field put its field editor at the field's left edge (x = 0). NSSearchFieldCell.edit/select puts the editor in whatever frame the cell is handed, and the cell handed it the field's full width. So "Search", the caret and typed text drew over the magnifier. At rest the cell draws in searchTextRect, which looked fine.

Fix

CmuxSidebarSearchFieldCell hands edit(withFrame:) and select(withFrame:) its searchTextRect(forBounds:). That rect is clear of the magnifier and cancel button, and the cell already centres it on one line. A bezeled field, as used in classic CmuxSidebarView / SidebarView, keeps AppKit's frame.

Tests

New HomeSearchGlassTests.theTypedTextStaysRightOfTheMagnifier at widths 180, 320 and 480. It makes the field first responder and checks that the field editor starts clear of the magnifier slot and at the placeholder's resting x, within 1 pt. AppKit's own text rect starts 1 pt inside its magnifier slot, even at rest. It then types text, which shows the cancel button, and checks again. Both editing tests now close their windows.

  • Red at the test-only commit 4a8fffa (base code), fleet step d737db5219984ee321a5a4fa: fails, box.minX 0.0 >= icon.maxX 27.0 and field editor starts at 0.0, the placeholder at rest at 26.0.
  • Green at head cccbc17 (feat-cmux-next b719608 merged in), fleet step 3d8c654c9d934bc0a4ecc16d; and before the merge at e118113, step 60d44ec621542d86cb71a96f (cmux-ci run --class light --script scripts/ci/package-test-lane.sh --ref e118113583e2c149b5bb50d0d2fa11e491c1d50c --arg suite --arg Packages/macOS/CmuxNext --arg HomeSearchGlassTests): 8 tests pass, no window leaks.
  • Compile-check at head (cmux-ci compile-check --app --package CmuxNext): job 8df07a60938c7f789c054c67 at cccbc17, CmuxNext and app pass.

Visual (real app, cmux DEV 27967-chief-search-align, fleet build 9063d03cc6c023f20b2b3a8d at e118113, rechecked on build cc2ae106e5741909a23b1be8 at the merged head cccbc17 (same ink columns); base build 08541a94af5cd86c73716c61 at dc38061)

Measured with debug.home.sidebar_fixture and debug.window_snapshot on the 2x window:

state magnifier ink x / centre y text ink start x / centre y filter centre y
base, rest 52–76 / 52.0 93 / 51.5 51.5
base, editing merged with text, 41–83 41 (over the magnifier) 52.0
fix, rest 52–76 / 52.0 93 / 51.5 51.5
fix, editing 52–76 / 52.0 93 / 51.5 52.0

Resizing the list while editing ({"search":true,"width":400}, then {"width":320}) keeps the same ink positions. Images are in the visual review comment.

Fleet capture

Capture tour-home at e118113583e2 (fleet build, capture mini, text checked for private data)

Light Dark
light dark

Viewed after image: yes
Visual verdict: Yes: in light and dark the magnifier, the "Search" placeholder and the round filter button sit on one line, and while editing the text stays beside the magnifier instead of over it, as in Messages.

Coordination

#27950 (Chief search content) touches the search results, not the field's cell or header layout. The two changes share no source files.

Observed / Not run

  • Observed: red/green of HomeSearchGlassTests on the fleet; real-app captures on the tagged dev build at this head.
  • Not run: real keyboard typing in the app (no focus-free route; the unit test types into the field editor); the full CmuxNext suite (the change is one cell class used by the Home list and the classic sidebars, and the bezeled path is unchanged); hover/press states (unchanged).

🤖 Generated with Claude Code

austinywang and others added 4 commits October 29, 2026 22:20
#27967, red)

Once the Home list's search field takes the keyboard, its field editor must
start where the placeholder sits at rest, right of the magnifier. Fails on
dc38061: the editor overlaps the magnifier.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tays right of the magnifier

#27967: once the Home list's search
field took the keyboard, CmuxSidebarSearchFieldCell handed AppKit's
edit(withFrame:) and select(withFrame:) a frame already cut to one 16 pt line.
AppKit lays out the magnifier slot and the text rect inside that frame, so
the field editor started over the magnifier and its line no longer matched
the filter circle beside it. The cell now passes the whole frame; its
searchTextRect, titleRect and drawingRect overrides still centre the line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AppKit puts an unbezeled search field's editor in the frame the cell is
handed; pass searchTextRect(forBounds:) so typed text and the caret sit
where the placeholder does at rest
(#27967).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@austinywang austinywang added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026
AppKit's search text rect overlaps its magnifier slot by a point at
rest as well, so the clear line is one point inside the slot.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review e118113: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr28021 -f ref=27967-chief-search-align -f platform=macos -f backend_mode=local

web and size checks: green

@blacksmith-sh

blacksmith-sh Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Found 2 test failures on Blacksmith runners:

Failures

Test View Logs
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)/
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)
View Logs
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)/
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)
View Logs

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

austinywang added a commit that referenced this pull request Oct 30, 2026
austinywang added a commit that referenced this pull request Oct 30, 2026
austinywang added a commit that referenced this pull request Oct 30, 2026
austinywang added a commit that referenced this pull request Oct 30, 2026
austinywang added a commit that referenced this pull request Oct 30, 2026
@austinywang

Copy link
Copy Markdown
Contributor Author

Visual review: NEEDS-HUMAN-TASTE (advisory)

Head e118113583e2 (base dc38061f72c6), reviewer claude-opus-5-5 visual-review subagent.

Merge: advisory, taste calls go to Leo through the feed; they never block the merge.

Real-app dark captures: while editing, the magnifier (ink x 52-76) and "Search" (ink x 93) now sit exactly where they sit at rest, level with the filter (centre y 51.5-52); the rest state is pixel-identical; light theme was not captured and the caret is not visible in the frame.

Evidence: Home search at rest; Home search editing (empty).

Criterion Result
evidence pass
intent pass
layout pass
contrast pass
unintended pass
frames pass
taste taste

Findings:

  1. note [intent] Fixed as intended. In the editing state the magnifier ink is at x 52-76 (centre y 52.0) and the placeholder ink starts at x 93 (centre y 51.5). Those are the same pixels as the rest state, and the placeholder no longer overlaps the magnifier the way the base editing frame does (x 41-83). The filter glyph centre is y 51.5, so the text sits level with it. Nothing to change. (01-after-home-search-editing.png at 40,36 140x32)
  2. note [intent] No insertion caret appears in either editing frame, probably because it caught the blink's off phase. The caret's position is not shown in the images; it rests on the PR's field-editor test (editor minX at or right of the magnifier). If you want image proof, capture one more editing frame with the caret visible or a typed character, and check that it starts near x 93. (01-after-home-search-editing.png at 85,36 20x32)
  3. note [contrast] Only the dark theme was captured. Dark contrast is fine: the gray placeholder and magnifier are legible on the dark field. Light theme is unverified, so capture the same two states in light to confirm the placeholder and the faint pill outline are still legible. (01-after-home-search-editing.png at 20,22 600x60)
  4. taste [taste] In the editing state the search becomes an outlined pill (thin gray stroke, about x 20-548). At rest there is no outline. This was already true on the base and this PR does not change it. Next to the Chrome, Claude and Codex search fields, judge whether this focus treatment is wanted. Also note that the filter turning blue comes from the fixture's Unread toggle, not from this change. Leo to judge. (01-after-home-search-editing.png at 18,20 534x64)
Contact sheet (before | after)

@austinywang

Copy link
Copy Markdown
Contributor Author

Correctness review (subagent), head e118113

Verdict: PASS. No blocking findings.

  1. Double inset or a second cut: none. On base, the editor landed at x = 0 (fleet step d737db5219984ee321a5a4fa), so super.edit/select uses the frame it is handed without adding a search-text inset. If super calls titleRect or drawingRect again, centred returns an already one-line rect unchanged.
  2. Bezeled fields are unaffected. editorFrame returns the given rect when isBezeled, and the classic sidebars use bezeled fields.
  3. Cancel button, long text, height. searchTextRect already excludes the cancel button, and long text scrolls inside the clip view.
    • Low: a fallback font with a taller line (emoji, CJK) could be clipped by the one-line rect. Dogfood note.
    • Low: the editor might move back when the field resizes mid-edit or the cancel button first appears.
      • Resolved: in the real app, debug.home.sidebar_fixture {"search":true,"width":400} and then {"width":320} with the field focused left the ink unchanged (magnifier 52–76 px, text from 93 px).
      • Resolved: the test now also types text, which shows the cancel button, and checks that the editor stays clear of the magnifier (commit e118113).
  4. Tests sound. isReleasedWhenClosed = false with defer { window.close() } keeps the suite from leaking windows.
    • Resolved: AppKit's own searchTextRect starts 1 pt inside its magnifier slot, even at rest (26 vs 27 pt). The clear line is therefore icon.maxX - 1. Base still fails by 26 pt.

An earlier review of a superseded head (0e03fa0, which removed the overrides) also passed. The real app then showed that fix did not work, which led to this change.

@austinywang

Copy link
Copy Markdown
Contributor Author

Head cccbc17: receipts

This head is a merge of feat-cmux-next b719608 into the branch, to pick up the base's package-deps fix (#28036). It does not change either PR file. The diff against the base is still CmuxSidebarSearchFieldCell.swift and HomeSearchGlassTests.swift, so the correctness review (PASS) and the visual review (advisory taste only) still apply.

  • HomeSearchGlassTests at this head: fleet step 3d8c654c9d934bc0a4ecc16d, 8 tests pass.
  • Compile-check (CmuxNext + app): job 8df07a60938c7f789c054c67, pass.
  • Dev build 27967-chief-search-align: job cc2ae106e5741909a23b1be8 at this head. In the real app (debug.window_snapshot), the ink columns while editing match those at rest: magnifier at 52–76 px, "Search" from 93 px, centred on the filter.
  • Fleet capture tour-home (light and dark): job 9063d03cc6c023f20b2b3a8d at e118113. The PR's files are the same there.
  • Merges cleanly with the latest feat-cmux-next, 65755f3.
  • Reds not caused by this PR:
    • nightly-guards: the release-notary digest test, which also fails on the base.
    • cmux-next swift test / fast tier: queued for a runner since 18:16Z. The swift test at e118113 ran HomeSearchGlassTests green; its other failures (ActionSurfaceParity, MainMenuPlacement, NativeTooltipGuard and others) are in suites this PR doesn't touch.

@austinywang austinywang added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Oct 30, 2026
@austinywang
austinywang merged commit 9c7e1db into feat-cmux-next Oct 30, 2026
198 of 202 checks passed
@austinywang
austinywang deleted the 27967-chief-search-align branch October 30, 2026 20:17
@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

This branch was previously deployed

1 inactive deployment
ci-routing-agent — cccbc177 Deployed Oct 30, 2026 by austinywang via wait for same-key tree publisher #84468
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant