Surface sidebar attention dots and flip Settings redesign default on#367
Merged
Conversation
Comment on lines
+35
to
+39
| case .appleIntelligence: | ||
| if !inputs.foundationModelAvailable { | ||
| categories.insert(.appleIntelligence) | ||
| categories.insert(.engineAndModel) | ||
| } |
Contributor
There was a problem hiding this comment.
categoriesNeedingAttention inserts .appleIntelligence whenever foundationModelAvailable is false, with no regard for the message string. calloutMessage(for: .appleIntelligence, ...) adds the extra guard !inputs.foundationModelMessage.isEmpty. If foundationModelAvailabilityService.userVisibleMessage ever returns an empty string while isAvailable is false — a defensive empty-string default, a service that hasn't yet loaded its message, etc. — the sidebar dot appears but the pane renders no callout, so the user sees an unexplained orange dot with no actionable copy below it. The dot condition and the callout guard should use the same criteria.
Suggested change
| case .appleIntelligence: | |
| if !inputs.foundationModelAvailable { | |
| categories.insert(.appleIntelligence) | |
| categories.insert(.engineAndModel) | |
| } | |
| case .appleIntelligence: | |
| if !inputs.foundationModelAvailable, !inputs.foundationModelMessage.isEmpty { | |
| categories.insert(.appleIntelligence) | |
| categories.insert(.engineAndModel) | |
| } |
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
1 participant
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.
Re-opened after base branch deletion. Rebased onto current main, build/tests/lint clean locally.
Greptile Summary
This PR completes the Settings redesign rollout: it surfaces per-pane orange attention dots in the sidebar (driven by a new pure
SettingsAttentionEvaluator), flips the redesign feature flag default totrue, and deletes the now-redundantPlaceholderPaneViewscaffold.SettingsAttentionEvaluator.swift): a stateless enum that maps a flatInputssnapshot to both the set of sidebar categories needing a dot and the per-pane callout message, making the rule unit-testable without AppKit.SettingsSidebarView): a 7 pt orangeCircleat the trailing edge of affected rows, with anaccessibilityLabel; theattentionCategoriesset is computed from live@ObservedObjectobservables inSettingsContainerViewso dots clear automatically on state recovery.SettingsCoordinator): usesobject(forKey:) as? Boolinstead ofbool(forKey:)to distinguish an absent key (defaulttrue) from an explicitfalseopt-out, with the legacy path retained for a one-defaults writerollback.Confidence Score: 4/5
Safe to merge with the dot-condition fix applied; the legacy rollback path remains intact.
The evaluator's dot condition for Apple Intelligence and its callout guard are not aligned on the message-emptiness check, which could produce a silent orange dot with no pane explanation. Everything else in the PR is clean.
Cotabby/Support/SettingsAttentionEvaluator.swift — specifically the Apple Intelligence branch in
categoriesNeedingAttention.Important Files Changed
trueusingobject(forKey:) as? Boolto distinguish absent key from an explicitfalseopt-out.attentionCategoriesdown from the observable graph to the sidebar; computed property drives reactive updates correctly.attentionCategoriesparameter and renders a 7 pt orange dot at the trailing edge of affected rows;.tagplacement on the outerHStackis correct forListselection.foundationModelAvailable == falseand `foundationModelMessage == "".Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[SettingsContainerView] -->|builds Inputs snapshot| B[SettingsAttentionEvaluator] B -->|Set of SettingsCategory| C[SettingsSidebarView] C -->|contains category| D{Show dot?} D -- Yes --> E[Orange 7pt Circle] D -- No --> F[Normal row] A -->|user opens pane| G[calloutMessage for category] G -->|non-nil| H[Pane callout banner] G -->|nil| I[Pane renders healthy] P[permissionManager] --> A Q[suggestionSettings] --> A R[foundationModelAvailabilityService] --> A S[runtimeModel.state.failureDetail] --> AReviews (1): Last reviewed commit: "Address Greptile feedback: drop empty ca..." | Re-trigger Greptile