Skip to content

Fix the staging screenshot flaking on the commit message caret - #716

Merged
hannesa2 merged 2 commits into
masterfrom
keep-the-caret-out-of-the-staging-screenshot
Sep 23, 2026
Merged

hannesa2 merged 2 commits into
masterfrom
keep-the-caret-out-of-the-staging-screenshot

Conversation

@goneng

@goneng goneng commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Take the keyboard focus out of the commit message field before the
staging view is captured, so the comparison stops reporting a caret
that no pull request drew.

PBGitCommitController's first responder is the commit message field, so
opening the staging view puts the keyboard focus there and its insertion
point blinks. Whether the caret lands in the capture then comes down to
when the screenshot is taken, and the comparison posts a difference
nobody made - most recently on #713.

  • Click the diff pane, which reads "No file selected" at that point and
    so takes the focus without selecting or changing anything.
  • Assert the diff pane is present first, so a view that never loaded
    fails with its own message rather than as a click into nothing.

Tab is not a way out here: the text view's doCommandBySelector: hands
the focus to a table, and focusTable: no-ops on an empty one, which is
what the fixture repo shows.

The second commit covers a different way a capture can differ from one
taken a moment later: a window whose app is not frontmost has its title
bar drawn dimmed. The tests assume the launch left the app in front
rather than checking, so the capture now waits for that, and asks for
activation only when the app is not already there - an activation
request would dismiss the open context menu the third test captures.

Test plan

  • Ran testStagingTabScreenshot locally six times before and after: the
    caret is in 5 of 5 captures before the change and 0 of 7 after
    (6 single runs plus a full-suite run), measured by scanning the commit
    message area for the caret's colour.
  • The whole GitXUITests suite passes, all four tests.
  • This pull request's own comparison should show the caret leaving the
    staging view once; comparisons after it should be quiet.
  • For the activation commit, reproduced the symptom by handing the
    activation to Finder just before the history capture: the title bar
    comes back dimmed and the traffic lights grey, and with the wait in
    place it matches an undisturbed run (title glyph brightness 105.9
    against 105.8).
  • Suite green again with both commits, and all three captures render
    the title identically.

@goneng
goneng force-pushed the keep-the-caret-out-of-the-staging-screenshot branch from 44b7596 to 5388f38 Compare September 22, 2026 05:28
Take the keyboard focus out of the commit message field before the
staging view is captured, so the comparison stops reporting a caret
that no pull request drew.

* Click the diff pane, which reads "No file selected" at that point
  and so takes the focus without selecting or changing anything.
* Assert the diff pane is present first, so a view that never loaded
  fails with its own message rather than as a click into nothing.
Wait for the app to be the frontmost one before capturing a window, so
a screenshot cannot record the dimmed title bar of an inactive window.

* Ask for activation only when the app is not already in front, since
  an activation request would dismiss an open context menu.
* Fail with a clear message when the app cannot be brought forward,
  rather than attaching a picture that cannot be compared.
@goneng
goneng force-pushed the keep-the-caret-out-of-the-staging-screenshot branch from 5388f38 to faaea0e Compare September 22, 2026 20:21
@goneng
goneng marked this pull request as ready for review September 22, 2026 20:31
@hannesa2

Copy link
Copy Markdown
Contributor

Screenshot differs: emulatorApi=arm64 with 1 screenshot(s)

setpoint (old) | diff | actual screenshot (new)

staging-view.pngscreenshot

@goneng goneng added the test Test related label Sep 22, 2026
@hannesa2
hannesa2 merged commit d8969c8 into master Sep 23, 2026
3 checks passed
@hannesa2
hannesa2 deleted the keep-the-caret-out-of-the-staging-screenshot branch September 23, 2026 04:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants