Repository navigation
fix(native): only keyboard focus holds the Swift tooltip open - #8441
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
|
/approve-spec 510f8df |
PR Analysis Report
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested for one sentence: the new INV1 text in tooltip.spec.md says the first show opens above the view, but placesAbove opens it below for a view within 20 points of the window's top, so the record and the code disagree there. Qualifying that sentence (for example, "opens above the view, or below it when even the gap does not fit under the space.s4 floor") is all that is needed; the click-focus fix itself matches core's useTooltip, and its test passes on macOS and the iOS Simulator. Optionally, the new test calls holdsByFocus and wantsBubble directly rather than update(), so moving the composed decision into one helper that update() calls would let it guard the fix.
[Automated review]
| On its first show, before the window's safe area and the bubble's own size | ||
| are known, the bubble opens above the view with no slide, and moves into | ||
| place on the next frame. |
There was a problem hiding this comment.
This doesn't hold near the window's top. On the first show the bubble's height and the safe top are still zero, so placesAbove puts the bubble above only when the view's top is at least space.s1 + space.s4 (20 points in the generated themes) below the window's top. Nearer the top it opens below and stays there, which is what INV1's own rule gives. Suggested wording: "the bubble opens above the view, or below it when even the gap does not fit under the space.s4 floor, with no slide, and moves into place on the next frame."
| @Test func aClickedViewsBubbleHidesWhenThePointerLeaves() { | ||
| typealias P = TooltipPresenter | ||
| // Focus a click brought does not hold the bubble once the pointer leaves. | ||
| let held = P.holdsByFocus(isFocused: true, focusIsKeyboard: false) | ||
| #expect(!held) | ||
| #expect(!P.wantsBubble(hasText: true, isHovering: false, isHoveringBubble: false, isFocused: held, focus: .tabStop)) | ||
| // Keyboard focus holds it. | ||
| #expect(P.holdsByFocus(isFocused: true, focusIsKeyboard: true)) | ||
| #expect(!P.holdsByFocus(isFocused: false, focusIsKeyboard: true)) | ||
| } |
There was a problem hiding this comment.
Optional: this composes holdsByFocus and wantsBubble the way update() does, but it never calls update(), so putting || (isShown && isFocused) back there would still pass. A static that returns the composed decision, which update() then calls, would let this test guard the fix.
…cord states its first show and scroll-on-view dismissal
510f8df to
4d90e40
Compare
|
/approve-spec 4d90e40 |
There was a problem hiding this comment.
Approval recommended: the click-focus fix now matches core's useTooltip, and both points from the earlier review are addressed, since the INV1 first-show sentence names the below case and the new test exercises showsBubble, the decision update() calls. The record edits carry your exact-head /approve-spec, and the Swift tests pass on macOS and the iOS Simulator. Nothing else is required here; as a follow-up, the native record and core could settle whether a keyboard-focused bubble should outlast a passing pointer, which core hides today.
[Automated review]
User need
On a Mac, a person clicks an avatar that has a tooltip, then rests the pointer on it. When the pointer moves away, the tooltip should go away, as it does on the web. Today the bubble stays up, because the focus the click left behind still counts as holding it.
Problem and solution fit
:focus-visiblecheck keeps such focus from opening or holding the tooltip.Current specification
architecture:native-swift-library-tooltip(packages/native/specs/swift/library/tooltip.spec.md), INV1, INV3, INV5.spec:AST-082FR3: a platform honors a core prop as core does, here core'suseTooltipfocus handling.Public delta and caller burden
None. The tooltip is internal.
Design and alternatives
holdsByFocus(isFocused:focusIsKeyboard:)is the one place that decides whether focus holds the bubble, andupdate()uses it. It replaces anisShown && isFocusedclause that let any focus hold a shown bubble.Evidence
TooltipPresenterTests.swift: a clicked view's bubble hides when the pointer leaves, and keyboard focus holds it.Open-work search
Searched open pull requests for
TooltipPresenter,tooltip.spec.md, andfocusIsKeyboard. Only the components that use the tooltip (#8306, #8323, #8328) touch it, and none change the presenter.Scope
Writing rubric results
Testing
swiftc -typecheckofAstryxNative,Astryx, and the tests passes;check:knowledgepasses.Every changed file is under
packages/native/.