-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Restore search-result row highlight in CmuxSettingsUI #5012
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
2152bad
Restore search-result row highlight in CmuxSettingsUI
azooz2003-bit bcce4a9
Make highlight anchor/state symbols internal to avoid app-target clash
azooz2003-bit e606214
Address review: terminate highlight schedule, consolidate state
azooz2003-bit 18fb6a2
Highlight section headers; fix Sidebar Branch Layout anchor
azooz2003-bit 643af42
Search lists only real rows; make every result scroll+highlight
azooz2003-bit 5d657f7
Restore 4 searchable settings the SPM table dropped
azooz2003-bit 018e325
Match search-result titles to their row labels
azooz2003-bit 1b53f90
Merge remote-tracking branch 'origin/main' into feat-settings-search-…
azooz2003-bit fc863e0
Pin scrolled-to section header to the top reliably
azooz2003-bit 5381781
Fix search-parity gaps found by independent audit
azooz2003-bit 07fbec5
Fix duplicate-anchor collisions; guard with a uniqueness test
azooz2003-bit 1747d3a
Make hibernation/custom-search sub-rows independently searchable
azooz2003-bit ff0ef46
Address round-4 code-quality findings
azooz2003-bit 08b3464
Drop dead-by-default conditional sub-field search results
azooz2003-bit bfa4124
Remove accidentally-committed dogfood screenshots
azooz2003-bit 1b5d319
Fix deep-row search hits scrolling to section top instead of the row
azooz2003-bit 46533bf
Realize the target section before scrolling to a deep row
azooz2003-bit 07a11b5
Eager detail stack + single scrollTo for search navigation
azooz2003-bit 0eceb3b
Add DEBUG :all search sentinel to enumerate every settings row
azooz2003-bit 956d0bb
Lazy-load the keyboard-shortcuts recorder list
azooz2003-bit e639f06
Add DEBUG window-state logging to SettingsWindowPresenter
azooz2003-bit b953994
Fix Settings window not reopening after close
azooz2003-bit beb34cc
Remove temporary window-state debug probes
azooz2003-bit File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
83 changes: 44 additions & 39 deletions
83
Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swift
Large diffs are not rendered by default.
Oops, something went wrong.
170 changes: 170 additions & 0 deletions
170
Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/SettingsSearchHighlight.swift
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,170 @@ | ||
| import SwiftUI | ||
|
|
||
| /// Drives the "flash the navigated-to row" affordance that the legacy | ||
| /// in-app settings window had and the SPM package initially dropped. | ||
| /// | ||
| /// When the user clicks a search hit in the sidebar, the detail scroll | ||
| /// snaps the matching row to the vertical center *and* the row pulses a | ||
| /// rounded accent-colored border for a few seconds so the eye can find | ||
| /// it. The pulse is owned by ``SettingsSearchHighlightState`` (set on | ||
| /// the settings root) and read by every ``SettingsCardRow`` through the | ||
| /// environment; a row participates by tagging itself with | ||
| /// ``SwiftUI/View/settingsSearchAnchors(_:)``. | ||
|
|
||
| /// The currently-highlighted anchor plus a monotonic token and the | ||
| /// instant the pulse began. `token` changes on every navigation so | ||
| /// re-navigating to the same row restarts the animation even when the | ||
| /// anchor id is unchanged; `startedAt` seeds the `TimelineView` fade. | ||
| // Intentionally `internal`, not `public`: the legacy in-app settings | ||
| // (`Sources/SettingsNavigation.swift`) declares a same-named | ||
| // `SettingsSearchHighlightState` plus matching `View` / | ||
| // `EnvironmentValues` extensions. The app target imports this package, so | ||
| // a `public` surface here collides with the legacy one and makes every | ||
| // unqualified use ambiguous. Nothing outside this package consumes these | ||
| // symbols, so keeping them internal removes them from the host namespace. | ||
| struct SettingsSearchHighlightState: Equatable, Sendable { | ||
| let anchorID: String? | ||
| let token: Int | ||
| let startedAt: Date? | ||
|
|
||
| init(anchorID: String?, token: Int, startedAt: Date?) { | ||
| self.anchorID = anchorID | ||
| self.token = token | ||
| self.startedAt = startedAt | ||
| } | ||
| } | ||
|
|
||
| private struct SettingsSearchHighlightStateKey: EnvironmentKey { | ||
| static let defaultValue = SettingsSearchHighlightState(anchorID: nil, token: 0, startedAt: nil) | ||
| } | ||
|
|
||
| extension EnvironmentValues { | ||
| /// The active search-result highlight. Defaults to "nothing | ||
| /// highlighted" so rows render inert outside the settings window. | ||
| var settingsSearchHighlightState: SettingsSearchHighlightState { | ||
| get { self[SettingsSearchHighlightStateKey.self] } | ||
| set { self[SettingsSearchHighlightStateKey.self] = newValue } | ||
| } | ||
| } | ||
|
|
||
| /// Resolves a row's dotted cmux.json path (declared via | ||
| /// ``SettingsConfigurationReview``) to the stable sidebar/search anchor | ||
| /// id the navigation layer scrolls to and highlights. Injected from the | ||
| /// settings root, which owns the built ``SettingsSearchIndex``. | ||
| private struct SettingsSearchIndexKey: EnvironmentKey { | ||
| static let defaultValue: SettingsSearchIndex? = nil | ||
| } | ||
|
|
||
| extension EnvironmentValues { | ||
| /// The settings search index, used by ``SettingsCardRow`` to map its | ||
| /// declared config paths to scroll/highlight anchor ids. `nil` when | ||
| /// a row is rendered outside the settings window (e.g. previews), in | ||
| /// which case the row simply doesn't anchor. | ||
| var settingsSearchIndex: SettingsSearchIndex? { | ||
| get { self[SettingsSearchIndexKey.self] } | ||
| set { self[SettingsSearchIndexKey.self] = newValue } | ||
| } | ||
| } | ||
|
|
||
| extension View { | ||
| /// Makes this view both `scrollTo`-addressable (via `.id` on the | ||
| /// first anchor) and eligible for the search-result highlight pulse | ||
| /// when any of `anchorIDs` matches the active highlight state. | ||
| @ViewBuilder | ||
| func settingsSearchAnchors(_ anchorIDs: [String]) -> some View { | ||
| let filteredAnchorIDs = anchorIDs.filter { !$0.isEmpty } | ||
| if let primaryAnchorID = filteredAnchorIDs.first { | ||
| self | ||
| .id(primaryAnchorID) | ||
| .modifier(SettingsSearchHighlightModifier(anchorIDs: filteredAnchorIDs)) | ||
| } else { | ||
| self | ||
| } | ||
| } | ||
|
|
||
| /// Eligible for the highlight pulse without claiming a `scrollTo` | ||
| /// `.id`. Used by section headers, whose enclosing section already | ||
| /// owns the `section:<raw>` scroll anchor — applying `.id` here too | ||
| /// would create a duplicate id and break scroll resolution. | ||
| @ViewBuilder | ||
| func settingsSearchHighlight(_ anchorIDs: [String]) -> some View { | ||
| let filteredAnchorIDs = anchorIDs.filter { !$0.isEmpty } | ||
| if filteredAnchorIDs.isEmpty { | ||
| self | ||
| } else { | ||
| self.modifier(SettingsSearchHighlightModifier(anchorIDs: filteredAnchorIDs)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Renders the pulsing accent border behind a row while it is the | ||
| /// active search-navigation target. A `TimelineView` with a finite | ||
| /// `.explicit` schedule drives the fade curve from `startedAt` so the | ||
| /// highlight ramps in, holds, then fades out without any timer or | ||
| /// `Task.sleep` in app code. | ||
| /// | ||
| /// The schedule is deliberately finite: it covers only the highlight | ||
| /// window (`pulseDuration`), so after the last frame the `TimelineView` | ||
| /// stops requesting updates and its display link goes idle. A plain | ||
| /// `.animation` schedule would keep firing at the display refresh rate | ||
| /// forever (drawing an invisible opacity-0 shape every frame) because | ||
| /// the highlight state isn't cleared until the next navigation. | ||
| private struct SettingsSearchHighlightModifier: ViewModifier { | ||
| @Environment(\.settingsSearchHighlightState) private var highlightState | ||
| let anchorIDs: [String] | ||
|
|
||
| /// Total length of the ramp-in + hold + fade-out pulse, in seconds. | ||
| private static let pulseDuration: TimeInterval = 5.9 | ||
| private static let frameInterval: TimeInterval = 1.0 / 60.0 | ||
|
|
||
| private func matches(_ state: SettingsSearchHighlightState) -> Bool { | ||
| guard let anchorID = state.anchorID else { return false } | ||
| return anchorIDs.contains(anchorID) | ||
| } | ||
|
|
||
| func body(content: Content) -> some View { | ||
| content | ||
| .background { | ||
| if matches(highlightState), let startedAt = highlightState.startedAt { | ||
| TimelineView(.explicit(frames(from: startedAt))) { context in | ||
| let opacity = highlightOpacity(at: context.date, startedAt: startedAt) | ||
| RoundedRectangle(cornerRadius: 8, style: .continuous) | ||
| .fill(Color.accentColor.opacity(opacity * 0.24)) | ||
| .overlay( | ||
| RoundedRectangle(cornerRadius: 8, style: .continuous) | ||
| .stroke(Color.accentColor.opacity(opacity), lineWidth: 2.5) | ||
| ) | ||
| .shadow(color: Color.accentColor.opacity(opacity * 0.24), radius: 8, x: 0, y: 0) | ||
| } | ||
| // Restart the animation when the user re-navigates to | ||
| // the same anchor: a changing token forces a fresh | ||
| // TimelineView identity so the schedule re-seeds. | ||
| .id(highlightState.token) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// One frame per display tick across the pulse window, ending after | ||
| /// the fade so the `TimelineView` schedule terminates. A row that | ||
| /// scrolls into view mid-pulse still renders the correct frame for | ||
| /// "now"; a row whose pulse already elapsed renders the final | ||
| /// (invisible) frame once and never schedules another update. | ||
| private func frames(from start: Date) -> [Date] { | ||
| let count = Int(Self.pulseDuration / Self.frameInterval) | ||
| return (0...count).map { start.addingTimeInterval(Double($0) * Self.frameInterval) } | ||
| } | ||
|
|
||
| private func highlightOpacity(at date: Date, startedAt: Date) -> Double { | ||
| let elapsed = date.timeIntervalSince(startedAt) | ||
| if elapsed < 0.14 { | ||
| return max(0, min(1, elapsed / 0.14)) | ||
| } | ||
| if elapsed < 5 { | ||
| return 1 | ||
| } | ||
| if elapsed < Self.pulseDuration { | ||
| return max(0, 1 - ((elapsed - 5) / 0.9)) | ||
| } | ||
| return 0 | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TimelineView(.animation)keeps a live display-link after fade-outOnce
elapsed >= 5.9thehighlightOpacityfunction always returns0, yethighlightedSearchAnchorIDis never cleared after the animation completes — only on the next navigation. Somatches(highlightState)staystrueand theTimelineView(.animation)keeps firing at display-refresh rate (~120fps on ProMotion), callinghighlightOpacityevery frame and drawingRoundedRectangleshapes withopacity(0)indefinitely. The display link for the targeted row remains live until the user clicks another search result or closes the window. Consider clearing the anchor id after the animation window via a one-shotTask { try? await Task.sleep(for: .seconds(6)); highlightedSearchAnchorID = nil }gated by the same token, or by deriving visibility fromelapsedin aTimelineSchedule.explicitschedule that terminates after the last keyframe.Rule Used: Flag new blocking or timing-based synchronization ... (source)