Repository navigation
Add surface transition signposts - #22
Open
dxnnystyles wants to merge 2 commits into
Open
dxnnystyles wants to merge 2 commits into
dxnnystyles wants to merge 2 commits into
Conversation
argahv
reviewed
Oct 23, 2026
argahv
left a comment
Owner
There was a problem hiding this comment.
Thanks, this is a clean way in, and the measurements in the description are really helpful. A couple of things against #14's criteria:
- Superseded transitions (
Sources/NotchFlow/AppModel.swift:792). Each interval only ends in its own.removedcompletion, so a transition interrupted by a newer one isn't closed or marked when it's replaced. Its interval runs on through the transitions that took over, which probably explains the 0.64–1.22 s intervals and the long one overlapping two later transitions. #14 asked for these to end correctly using the transition counter. For example, end with a superseded flag fromwindows?.isLatest(transition), or end the previous open interval when a newer transition begins. - Intervals that never end (line 798). When the completion is held back (Mission Control, occlusion), the 0.9 s fallback trims the panel but leaves the interval open. Ending it there too, marked as a fallback, with a flag so it only ends once, would keep the Instruments numbers clean.
- Small:
emitEventwith a static name already does nothing when recording is off, so theif signposter.isEnabledguards inWindowManager.swift(lines 235 and 266) can go.
Author
|
thanks for pointing these out! superseded transitions now close their intervals, and the fallback closes them too. added a registry so each interval can only end once + removed the extra |
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.
What this changes
Closes #14
Why
The motion timings were already defined, but there wasn't a way to measure the actual transitions in Instruments. This adds that without changing how the animations behave.
How it was tested
swift test --disable-sandbox— 196 passedpython3 -I Tests/test_bridge.py— 24 passedUniversal Release build passed
Project regeneration completed successfully; generated Xcode project updated.
Checked on macOS 26.5 with Instruments
git diff --checkpassedCaptured 12 transition intervals in Instruments, all with matching starts + ends
Tested opening, collapsing, and switching features mid-transition
Profiling used a Release-optimized build with the Debug command channel enabled for repeatable interactions.
Animated transitions measured around 0.64–1.22s from commit to completion. The longest overlapped with two later transitions, so I couldn't confirm what caused the delay.
No before/after screenshots are included because the change adds instrumentation without changing the visual interface.
Accessibility and motion
Privacy and security