Align and enlarge activation indicator#112
Merged
Merged
Conversation
Comment on lines
+15
to
+17
| /// Field-edge mode should visually touch the input's outside edge. A gap reads as accidental | ||
| /// padding because the icon is an affordance for the field itself, not for the surrounding UI. | ||
| private let fieldEdgeGap: CGFloat = 0 |
Contributor
There was a problem hiding this comment.
With
fieldEdgeGap = 0, the fallback-right path now also places the icon flush against the field's right edge. The intent for the preferred-left case (flush with the left edge) is clearly documented, but the symmetry on the right side is not. If a right-side fallback with zero gap ever looks visually abrupt — e.g. the icon immediately abutting text that overflows the field boundary — a dedicated fieldEdgeFallbackGap constant would make the asymmetric intent explicit and easy to adjust.
Suggested change
| /// Field-edge mode should visually touch the input's outside edge. A gap reads as accidental | |
| /// padding because the icon is an affordance for the field itself, not for the surrounding UI. | |
| private let fieldEdgeGap: CGFloat = 0 | |
| /// Field-edge mode should visually touch the input's outside edge. A gap reads as accidental | |
| /// padding because the icon is an affordance for the field itself, not for the surrounding UI. | |
| private let fieldEdgeGap: CGFloat = 0 | |
| /// When the indicator must fall back to the right side of the field (left-edge-hugging inputs), | |
| /// a small gap avoids abutting text that overflows the field boundary. Set to 0 to match the | |
| /// flush-edge intent, or raise to a small value (e.g. 4) if right-side fallback looks crowded. | |
| private let fieldEdgeFallbackGap: CGFloat = 0 |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
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.
Summary
Makes the field-edge Tabby activation indicator larger, visually flush with the focused input edge, and slightly lighter so the black app icon reads less heavy against text fields.
This keeps the change inside
ActivationIndicatorController: the positioning gap is now zero for field-edge mode, the icon renders at 20pt instead of 16pt, and a small SwiftUI brightness adjustment softens the black without adding a new asset pipeline.Validation
git diff --check-> exit 0xcodebuild -project tabby.xcodeproj -scheme tabby -destination 'platform=macOS' build-> BUILD SUCCEEDEDLinked issues
None.
Risk / rollout notes
The indicator now touches the input frame edge exactly in field-edge mode. If a host app reports a slightly oversized AX input frame, the icon will align to that reported frame because Tabby intentionally treats Accessibility geometry as the source of truth.
Greptile Summary
This PR polishes the
fieldEdgeIconactivation indicator: the positioning gap is reduced from 6 pt to 0 so the icon sits flush with the focused input's edge, the icon grows from 16 pt to 20 pt, abrightness(0.16)modifier lightens the black app icon, and the corner radius / shadow are nudged to match.horizontalGap(6) is replaced byfieldEdgeGap(0), affecting both the preferred-left and fallback-right computations infieldEdgeIconOrigin, making the indicator flush on both sides.FieldEdgeIconIndicatorViewgains.brightness(0.16)between.frame()and.clipShape(), which uniformly raises every color channel — safe for the current dark icon but worth keeping in mind if the icon gains colorful regions in the future.Confidence Score: 4/5
Safe to merge — all changes are confined to visual presentation with no impact on logic, state, or Accessibility geometry handling.
The diff touches only layout constants and SwiftUI view modifiers inside a single private view. There are no data-flow changes, no new async paths, and the positioning arithmetic is identical apart from substituting 0 for the old 6-pt gap. The two observations are style-level: the right-side fallback silently inherits the zero gap without documentation, and the
.brightnessmodifier sits after.frame()rather than closer to the image declaration — neither affects runtime correctness.No files require special attention beyond a quick visual sanity-check of the right-side fallback rendering in
ActivationIndicatorController.swift.Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["show(mode:caretRect:inputFrameRect:)"] --> B{mode?} B -- hidden --> C[hide] B -- caretAnchor --> D[caretAnchorOrigin] B -- fieldEdgeIcon --> E[fieldEdgeIconOrigin] E --> F{"inputFrameRect\navailable & non-empty?"} F -- yes --> G[anchorRect = inputFrameRect] F -- no --> H[anchorRect = caretRect] G & H --> I["preferredLeftX =\nanchorRect.minX − contentSize.width\n− fieldEdgeGap (0)"] I --> J["fallbackRightX =\nanchorRect.maxX\n+ fieldEdgeGap (0)"] J --> K{preferredLeftX ≥\nvisibleFrame.minX + screenInset?} K -- yes --> L[use preferredLeftX] K -- no --> M[use fallbackRightX] L & M --> N[clamp X & Y to screen bounds] N --> O[set panel frame & order front] D --> P[center on caret X, prefer below line] P --> NComments Outside Diff (1)
tabby/Services/UI/ActivationIndicatorController.swift, line 216-222 (link).brightness(0.16)adds 0.16 to every color channel before clipping. Placing it before.clipShape()means the brightened pixels outside the rounded rectangle are computed but then discarded by the clip, which is harmless. However, inserting it between.frame()and.clipShape()rather than right afterImage(nsImage:)is slightly non-idiomatic — typically image-level color adjustments come before layout modifiers so the intent (modify the image, not the layout box) is clear at a glance.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (1): Last reviewed commit: "Align activation indicator with field ed..." | Re-trigger Greptile