fix(popover): point the indicator at the anchor - #1920
Open
bataevvlad wants to merge 8 commits into
Open
bataevvlad wants to merge 8 commits into
bataevvlad wants to merge 8 commits into
Conversation
The anchor stays wrapped in MeasureElement from the first render so the tree keeps its shape, but a closed popover measured it on every layout, undoing the lazy mount from 118f94a. MeasureElement gains `enabled`; Popover enables it on the first show, where the forced measurement supplies the anchor frame and the content waits off screen until then.
The five icon galleries added ~1000 accessibility nodes. agent-device snapshots stop at 1500 nodes and walk the root view first, so anything presented afterwards in the same window, i.e. every React Native Modal, was cut off and modal-nested.ad could not find its Select. Hide the gallery grids from accessibility: the parity sweep reads screenshots. autocomplete.ad reached its section with a single 'scroll bottom', which now stops fifteen passes short in the galleries. Scroll from the top like modal-nested.ad and move the field above the keyboard: the floating list opens below the field and was tapped through the keyboard otherwise.
The placement bounds were the whole window, so an Autocomplete near the bottom of a screen opened its list under the software keyboard. The keyboard height now shrinks the bounds: a list that does not fit below the field flips above it, an anchor hidden by the keyboard gets its content clamped right above the keyboard, and the placement is redone when the keyboard shows or hides while the popover is open. Nothing told a popover that its anchor moved while a non-blocking overlay (Autocomplete) was open, so the list stayed where the field used to be while the screen scrolled. The anchor is measured once per frame while such a popover is visible, through a new imperative handle on MeasureElement; the content follows the anchor and goes off screen while the anchor is outside the window.
The hosted macOS runner needs about twice the local time per step: the 46 scrolls of modal-nested.ad alone took 87 s there, so the script hit its 180 s budget before reaching the modal.
Every popover subscribed to the keyboard events and re-measured its anchor on each one, so a screen of closed Selects made hundreds of native measurements per keyboard show or hide, and the on-demand measure() ignored MeasureElement's enabled flag. Subscribe while visible, read a keyboard that is already up when opening, follow iOS keyboardWillChangeFrame, and make measure() a no-op while disabled.
When the content is moved to stay on screen (a wide tooltip anchored near an edge, or the start/end placements the service falls back to), the indicator stayed at the content's own centre or fixed offset and no longer pointed at the anchor; on a narrow tooltip it could sit past the content edge and disappear. Popover now derives the cross-axis distance between the anchor centre and the placed content centre and PopoverView translates the indicator by it, clamped inside the content edges.
bataevvlad
force-pushed
the
fix/popover-indicator-anchor
branch
from
September 30, 2026 09:35
3edaa02 to
44db62a
Compare
This branch has not been deployed
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.
Stacked on #1919 (which is stacked on #1914 and #1918); only the last commit is new here.
A wide
Tooltipon a button near a screen edge is moved back inside the window (or placed with thestart/endvariant of its placement), but the arrow stayed at the tooltip's own centre or at the fixed start/end offset, so it no longer pointed at the button; on the medium-width case it ended up past the content edge and was not visible at all.usePopoverMeasurementnow derives, along the axis the content runs across, the distance between the anchor centre and the placed content centre, clamped so the arrow stays inside the content's edges (INDICATOR_EDGE_MARGIN).PopoverViewtakes it asindicatorOffset, aligns the indicator from the centre and translates it by that amount before the orientation rotations. Without the propPopoverViewbehaves as before.Verification: iPhone 17 simulator, showcase
TooltipEdge: the long tooltip on the right-aligned button, the medium one (arrow previously missing) and the left-aligned one all show the arrow under/over their own button. Newpopover.indicator.spec.tsxcovers centred, pushed-back, clamped and side placements. Full jest suite, lint andtypecheck:allpass.iOS verification (2026-09-28)
iPhone 17 simulator (iOS 26.2), debug build on Metro from a clean worktree of this branch, driven with agent-device.
section-TooltipEdge: the right-aligned long tooltip, the right-aligned medium one (arrow previously missing) and the left-alignedbottomone all show the arrow under their ownTIPbutton; the centred one shows it above. Screenshots per case.PopoverandTooltipsections: content anchored to the button, arrow centred, close on backdrop.yarn e2e:iosagainst a Metro serving this branch (flags injected into a scratch copy; an earlier run reported here loaded another checkout's bundle from:8081):smoke.ad,icon-touchable.adandmodal-nested.adpass (modal-nested.adfails on master); the rewrittenautocomplete.adfails at step 75, its fixedscroll down 0.3chain ending short of the Autocomplete section.Android verification (2026-09-28, second pass)
Pixel 7 emulator (API 34), debug build on Metro from a clean worktree of this branch, driven with agent-device.
section-TooltipEdge: all four tooltips (right long, right medium, left longbottom, centre) draw the arrow at their ownTIPbutton.Popover/Tooltip: anchored, arrow centred, close on outside tap.popover-android.adis stale on master (its fixed scroll count stops at the List section) and was not a branch check; test(e2e): reach showcase sections through a deep link #1932 replaces the scrolls with a section deep link. The Android replay line previously reported here had also loaded another checkout's bundle from:8081.Update (2026-09-30)
Re-stacked on the updated #1919 (its own commit cherry-picked unchanged as
44db62a6). Popover specs 141/141, full suite 1904 tests,typecheck:allclean; iPhone 17TooltipEdgemedium tooltip still points at its button.