Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
meta-cla
Bot
added
the
CLA Signed
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: the min-height: fit-content change works in WebKit (a capped sheet with content placed directly in it now ends 16 px below its last line instead of 0 px, and Chromium is unchanged), but two points need addressing before approval.
- The flex-row dialog host carried from #8534 contradicts the current
component:BottomSheetrecord, whose element jobs say the dialog holds the panel "with its own grid"; please amend that line through the spec gate (an engineering owner can approve it with/approve-spec <head SHA>) or keep the grid. - Both changesets claim Safari fixes, but the readings come from Playwright WebKit, and
spec:AST-013FR10 asks for evidence from Safari itself; please add iOS Safari readings (including one behind the "about 40 ms" relayout figure) or narrow the wording to what was measured.
[Automated review]
| display: 'grid', | ||
| alignContent: 'end', | ||
| justifyItems: 'center', | ||
| display: 'flex', |
There was a problem hiding this comment.
This makes the open dialog a flex row, but the current component:BottomSheet record (Performance and resources, element jobs) says the dialog "holds the panel at the block end with its own grid". Please amend that line through the spec gate or keep the grid host; the layout itself is unchanged in Chromium and WebKit.
There was a problem hiding this comment.
The flex row is no longer part of this pull request: it now sits on main's grid host, where the fix and its guard pass in both engines. Whether the row stays is #8534's own owner question.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] BottomSheet: in Safari, a sheet at a set height whose content sits directly in it keeps its bottom padding at the end of its scroll, instead of ending flush with the last line. |
There was a problem hiding this comment.
The fix holds in Playwright WebKit (0 px to 16 px at the scroll end of a capped direct-content sheet), but spec:AST-013 FR10 says Playwright WebKit does not prove Safari behavior. Please add an iOS Safari reading before and after, or describe the fix in terms of what was measured.
There was a problem hiding this comment.
Narrowed in 9bfb74a to what was measured, Playwright's WebKit. iOS Safari readings before and after are being taken, and the wording will name Safari once they land.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] BottomSheet: in Safari, an update inside an open sheet no longer holds up the page about 40 ms longer than it should. The open sheet's dialog places its panel with a flex row instead of a grid; the layout is the same. |
There was a problem hiding this comment.
The "about 40 ms" Safari figure needs a measurement from Safari itself under spec:AST-013 FR10; the commit messages report it from a phone-sized WebKit page.
github-actions
Bot
added
the
needs:spec-owner-review
…Kit's scroll range The panel's content box takes min-height: min-content so it grows with its content past a fixed-height viewport. WebKit ignores min-content on a block's min-height, so on a sheet at a set height the box stayed its body's height: content placed directly in it overflowed the box, and its block-end padding fell out of the scroll range, ending the scroll flush with the last line. fit-content is the same size in the block axis and WebKit honours it. A guard reads, at each sheet's scroll end, how far the content clears the range's end: on main a capped sheet with direct content reads 0 px in WebKit, and 16 px (the content padding) with the fix. iOS Safari on the 26.2 simulator reads the same, 0 px before and 16 px after; Chromium was already right. The guard also covers two hug sheets with direct content.
9bfb74a to
982a837
Compare
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Recommending approval: switching the BottomSheet content box to min-height: fit-content keeps its bottom padding inside the scroll range of a set-height sheet with direct content in WebKit (0 px to 16 px at the scroll end), with no change in Chromium. The new scroll-end guard fails on the base in WebKit and passes on this head, and the dialog-host and Safari-wording points raised on the earlier revision are resolved. Firefox 151 still ends such a sheet flush, unchanged from main and outside this fix.
[Automated review]
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewComponent Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included. Bundle Size SummaryGzip figures measure each package's entry file, not the code an import pulls in.
Accessibility AuditStatus: No accessibility violations detected. Visual Regression55 of 502 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 35 more. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
|
Posted by Cindy's agent on her behalf: closed and reopened only to restart this PR's CI on the current workflow after tonight's queue-wide cancel; nothing in the PR changed. |
Review only this PR's change: vjeux/astryx@8a13f3d...982a837
Why
In Safari on iPhone, and in WebKit generally, a bottom sheet at a set height (
capped,tall, snap points) whose overflowing content sits directly in it, without a Section, scrolls to an end flush with the last line: the content padding below it is gone, so the last line sits against the sheet's bottom edge, under the home indicator. Chrome keeps the 16 px.What led to it
This comes from the work to make Astryx's touch components behave as native iOS ones do (sheets, menus, gestures and their motion). The content box is
height: 100%; min-height: min-content. On a set height100%resolves, and WebKit ignoresmin-contenton a block's min-height, so the box stays the body's height (522 px against 1,760 px of content) and its bottom padding falls out of the scroll range.Options and tradeoffs
component:BottomSheetFR3 assigns to the content box.height: 100%: loses the definite height that content sized in percentages relies on.min-height: fit-content: the same size in the block axis, and WebKit honours it.Why this choice
It is a one-value change that keeps the box's role and its percentage-height basis, and it makes WebKit lay the box out exactly as Chrome already does.
How to check it
Evidence
Scope