Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
the
needs:design-review
|
AI review status for this pull request.
|
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes on both pull requests in this stack. A pull request lands only when it and every pull request below it are approved.
#8085: request changes
- On its own it adds
useTouchHitAreaandtouchHitAreaStyles.layerbut leaves InfoTip's own callback ref and::afterbeside them, so land it together with #9916 or fold #9916 into it.
#9916: request changes
- Inside a divided
Listrow, InfoTip's button inherits the row's--_hit-border-bottom, so the shared layer moves its touch band 1 px down (bottom at −11.5 px against a 10.5 px measured inset under touch in Chromium), giving the strip right under the icon to the row and reaching 1 px past the half-gap clamp incomponent:InfoTipAR1; keep that variable off descendants, for example by declaring it on the row's own::after, and add an InfoTip-in-divided-List test.
Whole stack: fails on the InfoTip finding above. Every other delta matches component:List FR10, spec:AST-094 FR42 and InfoTip AR1, and mouse behavior is unchanged. Two optional comment trims are noted inline.
[Automated review]
| focusOutlineStyles.focusVisible, | ||
| styles.trigger, | ||
| styles.hitArea, | ||
| touchHitAreaStyles.layer, |
There was a problem hiding this comment.
Inside a divided List row this button inherits --_hit-border-bottom from the row (ListItem.tsx sets it on the <li>, and custom properties inherit). This layer's bottom edge then becomes -(inset + 1px): under touch in Chromium it measures −11.5 px against a 10.5 px inset. The strip right under the icon goes to the row, and the band runs 1 px past the half-gap clamp in InfoTip AR1.
| }, | ||
| // The touch hit layer's band starts past the divider, the edge of the box | ||
| // its insets are measured from (utils/touchHitArea.stylex.ts). | ||
| '--_hit-border-bottom': { |
There was a problem hiding this comment.
Declared on the <li>, this reaches every descendant, so an InfoTip's shared layer inside the row adds the divider width to its own band. Declaring it on this row's own ::after, its only intended reader, keeps it local.
| // Under a coarse pointer the button takes taps out toward 44 px, on the | ||
| // shared touch layer. |
There was a problem hiding this comment.
Non-blocking: this restates useTouchHitArea's own JSDoc, and the call says it. Suggest deleting the comment.
…rs leave room Under a coarse primary pointer, an interactive List row gets an invisible hit layer on its ::after. The shared touch hit area measures each row and grows the layer toward 44 px, stopping at half the gap to the nearest neighboring row or control. Rows look and lay out as before, and a mouse is unchanged. A List link row's padding and band follow its link under a finger, through the shared clickable-container guards: a control inside the row and the click that ends a text selection keep their own behavior. A standalone Item keeps its anchor as its only link target. Under a finger a List isolates its rows, so a raised band or swipeable row stays below a page-level sticky header. A row's watch of the shared hit area owns the touches on its content, so a tap on a swipe panel's entry beside an open row stays the entry's. Other watches, such as Switch's grown input, keep the nearer-neighbour rule. A stopped watch clears the insets it wrote. A divided row declares its divider's width on its own layer, so content inside the row never inherits it. A delegate row renders its pressable-container marker itself, so a row that stops delegating drops it.
The With end controls story's drill-in chevrons take rtlStyles.mirror, so they point to the row's end under right-to-left. Story only.
InfoTip's button kept its own copy of the List rows' touch target: a callback ref over the shared hit area and its own ::after layer. It now uses useTouchHitArea and touchHitAreaStyles.layer, so one path serves both. A stopped watch clears the button's insets, a measurement waits for motion around the button to end, and the layer exists only under a coarse pointer.
c15fab5 to
ced35b3
Compare
There was a problem hiding this comment.
Approving #9916 pending CI; #8085 below it still needs to land together with it. A pull request lands only when it and every pull request below it are approved.
#9916: approve
- The InfoTip band inside a divided
Listrow now ends at its measured inset, and the new story and browser test cover that case.
#8085: request changes
- On its own it adds
useTouchHitAreaandtouchHitAreaStyles.layerbut leaves InfoTip's own callback ref and::afterbeside them, so land it together with #9916 or fold #9916 into it.
Whole stack: passes; every delta matches component:List FR10, spec:AST-094 FR42 and component:InfoTip AR1, and mouse behavior is unchanged.
Approved pending CI: CI has not run on this head yet, because the pull request is a draft from a fork; it runs once it is ready for review and a maintainer adds the ci:run label. One optional comment trim is noted inline.
[Automated review]
| // Under a coarse primary pointer, a host's hit target grows toward the touch | ||
| // target where its neighbors leave room. Nothing moves and nothing repaints. | ||
| // utils/hitArea.ts measures how far on each side. It stops at half the gap to | ||
| // the nearest neighboring target, and at 0 where they touch. | ||
| // | ||
| // The layer is the host's `::after`, grown past the host's edges by those | ||
| // insets. The insets are 0 until the first measurement, so first paint and | ||
| // server rendering never overlap a neighbor. The host is the layer's | ||
| // containing block, so a host that is not positioned sets `position: | ||
| // relative`. The insets start at the host's border box, but the layer's | ||
| // offsets start at its padding box. So a host that draws a border declares | ||
| // its width on that side (`--_hit-border-{top,right,bottom,left}`, 0 by | ||
| // default). It declares it on its own `::after`, so content inside the host | ||
| // never inherits it. A divided list row declares its divider's width. | ||
| // | ||
| // A clip path cuts the host's own box out of the layer. So the host's content | ||
| // and controls keep their taps, and a tap in the band goes to the host. The | ||
| // layer has no z-index of its own. A later sibling that is positioned with no | ||
| // z-index paints and hit-tests over it. Inside a List, which isolates its | ||
| // rows, the layer takes `z-index: 1`, so a later row cannot cover it. A host | ||
| // outside such a container raises nothing, so its band never passes a | ||
| // page-level sticky header. The host itself keeps no isolation and no | ||
| // z-index, so at rest it is no stacking context. A swipeable row is a | ||
| // stacking context, so it raises itself. | ||
| // | ||
| // Under a finger the host's `::after` is this layer, so decorate the host | ||
| // with a child. The insets are measured in the viewport, so the inline edges | ||
| // are physical: a left inset grows the left edge in either direction. StyleX | ||
| // evaluates property names at build time, so they are spelled out, and | ||
| // touchHitArea.test.tsx checks them against HIT_INSET_PROPERTIES. |
There was a problem hiding this comment.
Optional (comment style): this 30-line block repeats much of the file header. It could keep only the constraints, for example: "The insets start at the host's border box. The offsets start at its padding box. A host with a border declares its width on its own ::after. Under a finger the host's ::after is this layer."
Stack: #8085 → #9916, #9894
Stacked on #8085: review only this pull request's last commit, vjeux/astryx@3d0e824...ced35b3. It moves to main once #8085 merges.
Primary intent
InfoTip's button takes taps under a finger through the same touch hook and layer as List rows, so one path serves both. This is a refactor that also changes InfoTip's touch target a little (below). So neither the maintenance template (no behavior change) nor the bug-fix template (one restored defect) fits.
User or maintainer impact
Maintainers keep one copy of the coarse-pointer touch target instead of two. On a phone, InfoTip's button target still grows toward 44 px where its neighbors leave room, and never into a neighbor's. It now clears its insets when the button stops taking taps, it waits for motion around the button to end before it grows, and its layer exists only under a coarse pointer. A mouse sees no change.
Problem and solution fit
#8085 adds
useTouchHitAreaandtouchHitAreaStyles.layerfor List rows. InfoTip kept its own copy: a callback ref overwatchHitAreaand its own::afterlayer. Two copies drift: the shared layer's stop-time clearing, motion wait and border offset never reached InfoTip. This change deletes InfoTip's copy and uses the shared hook and layer. The button is alreadyposition: relativewith no border, so it needs no border variables.Authority
component:InfoTipFR1 and AR1, both current: under a coarse pointer the button reaches 44 px through an invisible hit area where its neighbors leave room, and the hit area never overlaps a neighbor's. No record changes.Change and evidence
InfoTip.tsx:useTouchHitArea(true)replaces the callback ref, andtouchHitAreaStyles.layerreplaces the::afterstyle.utils/touchHitArea.tsandutils/touchHitArea.stylex.ts: their headers name InfoTip as a user.InfoTipTouch.a11y.chromium.spec.ts: inside a divided List row the button's layer reaches its measured inset and no further. It fails while the row declares its divider width on itself, which feat(List): a row's touch target grows toward 44 px where its neighbors leave room #8085 now keeps on the row's own layer.watchAtRestinutils/touchHitArea.ts), tested intouchHitArea.test.tsx("drops a measurement taken while the element’s parent moves, and watches afresh once the motion ends"). It now covers InfoTip's button through the hook.Scope
Testing
What was run at this head: InfoTipTouch 4/4 in Chromium, including the new divided-List case. That case fails when the row declares its divider width on itself (the button then inherits 1px). Lint, prettier, tsc and changeset coverage pass, and the public text check passes on the commit and this body. The rest of the tree is #8085's head, gated there.