Skip to content

Smooth shelf dismissal animation - #4

Merged
tarikbc merged 2 commits into
tarikbc:mainfrom
brzvsk:fix/shelf-dismiss-animation
Sep 11, 2026
Merged

tarikbc merged 2 commits into
tarikbc:mainfrom
brzvsk:fix/shelf-dismiss-animation

Conversation

@brzvsk

@brzvsk brzvsk commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • keep Copy active until the shelf is fully ordered out, so Liquid Glass does not resample a different app backdrop during the fade
  • leave the hidden panel at zero opacity instead of resetting it in the same run-loop turn as orderOut
  • make dismissal slightly smoother with a 180 ms ease-in-out fade and a smaller 18 pt slide
  • run paste and open-URL actions after dismissal and focus restoration; overlapping actions join the same close and are drained in FIFO order
  • preserve the immediate path when Reduce Motion is enabled

Why

On macOS 26, the shelf could flicker for a frame or appear to disappear abruptly. An Instruments Animation Hitches trace did not show application-side rendering hitches: the transition is handled by the compositor. The visible flash came from changing the active app behind the live glass while it was still on screen, combined with resetting the panel opacity immediately after ordering it out.

Verification

  • built the macOS app successfully with xcodebuild
  • ran all 184 CopyCore tests successfully
  • exercised eight repeated show/hide cycles without a crash or stuck close state
  • manually verified the animation, Escape, click-away, paste, and focus restoration on macOS 26.5.2
  • ran a focused independent review; its overlapping-action finding was fixed, and the linked closure review returned ready with zero findings

brzvsk and others added 2 commits September 10, 2026 22:10
Only `finishHide` drains `pendingHideCompletions`, and a close that is
superseded never reaches it: `show()` bumps `closeToken`, so the fade's
completion handler returns early. The queued paste then survived and fired
at the *next* close instead.

Press Return, re-open the shelf inside the 180 ms fade, then close it
again, and an item chosen seconds earlier pasted into whatever app
happened to be frontmost. `show()` now drops the queue along with the
close it belonged to, which is what re-summoning the shelf means.

Modelled the token bookkeeping to confirm both halves, since the app
target carries no tests.
@tarikbc
tarikbc force-pushed the fix/shelf-dismiss-animation branch from 2201bbf to ef79abb Compare September 11, 2026 01:11
@tarikbc

tarikbc commented Sep 11, 2026

Copy link
Copy Markdown
Owner

The diagnosis here is good and I had not thought about it: activating the previous app while the Liquid Glass panel is still fading does resample its backdrop mid-flight. Moving activate() after orderOut is the right shape, leaving alpha at zero is safe because show() always establishes its own zero state, and you converted all three call sites rather than one.

I pushed one commit for a bug the rework introduced.

A superseded close stranded the paste it was carrying. Only finishHide drains pendingHideCompletions, and a superseded close never reaches it, because show() bumps closeToken and the fade completion returns early. The queued closure survived and fired at the next close:

  1. Press Return. The paste is queued, the 180 ms fade starts.
  2. Press ⇧⌘V inside those 180 ms. show() bumps the token.
  3. The old fade lands, sees a stale token, returns. finishHide never runs.
  4. The next close drains the queue, and an item chosen seconds ago pastes into whatever app is now frontmost.

show() now drops the queue along with the close it belonged to, which is what re-summoning the shelf means. Since the app target has no tests, I modelled the closeToken / isHiding / queue bookkeeping separately to confirm both the bug and the fix.

One thing I left alone deliberately: the paste keystroke now fires about 330 ms after Return rather than 150 ms, because it waits out the fade. The activation margin itself is unchanged at 150 ms, so reliability does not drop, and that latency is the trade this PR is making on purpose. Worth watching in daily use.

Builds clean. Merging.

@tarikbc
tarikbc merged commit 7f8882c into tarikbc:main Sep 11, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants