Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
There was a problem hiding this comment.
This stack needs an engineering owner's decision before it can be approved: no current spec governs NavigationStack's large titles, and the draft spec:AST-094 it carries, including its proposed changes to the current theme-tokens, interaction-modality, buttons, user-states and AST-025 records, has no owner approval. An engineering owner can record that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, these need fixing:
- The bar title shows at full opacity over a visible large title for about half a second when a scrolled large-title screen is pushed again and when popping back to a large-title root from two levels down, so clear a screen's collapsed flag when its card unmounts and ignore observer reports while a card is
content-visibility: hidden. - After a pop that lands one or more levels deep, the screen underneath stays
content-visibility: hiddenwith no bar becauseunderKeykeeps the finished pop'sunder, so a back swipe drags in an empty card. - The diff against
mainis about 29,000 authored lines from 28 commits, so land or drop the predecessor pull requests first and let this one become its own large-title change.
[Automated review]
| {root: card, threshold: 0}, | ||
| ); | ||
| observer.observe(title); | ||
| return () => observer.disconnect(); |
There was a problem hiding this comment.
When this card unmounts, its key stays in collapsedTitles, so pushing the same entry again renders the bar title at full opacity until the new observer reports (measured in Chromium: 1.00, fading to 0 over about 490 ms) — the flash react-component-runtime INV1 rules out. A newly mounted card starts at scroll 0, so dropping the key here fixes it, for example by also calling onLargeTitleCollapse(model.key, false) in this cleanup.
| touchAction: 'pan-y pinch-zoom', | ||
| }, | ||
| cardHidden: { | ||
| contentVisibility: 'hidden', |
There was a problem hiding this comment.
While a card is content-visibility: hidden, the large-title observer reports the title as not intersecting, so the root is marked collapsed while it is two levels down, and popping back to it shows the bar title over the large title for about half a second (measured: opacity 1.00 at the pop, 0.01 at about 480 ms). Ignoring observer entries while the card is hidden, and keeping the last value, avoids it.
| const underKey = | ||
| view.transition.kind !== 'none' | ||
| ? view.transition.under | ||
| : (view.models.at(-2)?.key ?? null); |
There was a problem hiding this comment.
After a pop finishes, transition.under is the new top screen, so the screen actually underneath gets isHidden (content-visibility: hidden) and no bar layer. In the Settings story, depth 1 reached by popping back from About leaves the root's rows unrendered (checkVisibility() false), so the next back swipe drags in an empty card. Taking the under screen from view.models.at(-2) once the transition has finished fixes it.
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 |
f28c941 to
62e58c4
Compare
There was a problem hiding this comment.
This stack needs an engineering owner's decision before it can be approved: no current spec governs NavigationStack's large titles, and the draft spec:AST-094 it carries, including its proposed changes to the current theme-tokens, interaction-modality, buttons, user-states and AST-025 records, has no owner approval. An engineering owner can record that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, these still need fixing:
- The bar title shows at full opacity over a visible large title for about half a second when a scrolled large-title screen is pushed again and when Done returns to a large-title root from two levels down, so clear a screen's collapsed flag when its card unmounts and ignore observer reports while a card is
content-visibility: hidden. - After a pop that lands one or more levels deep, the screen underneath stays
content-visibility: hiddenwith no bar becauseunderKeykeeps the finished pop'sunder, so a back swipe drags in an empty card. - The diff against
mainis about 29,000 authored lines from 29 commits, so land or drop the predecessor pull requests first and let this one become its own large-title change.
[Automated review]
| follows. | ||
| - **FR7 — Exits animate as UIKit's do.** A touch presentation MUST play the | ||
| exit its UIKit counterpart plays. **Proposed; pending owner review:** this | ||
| departs from the draft `spec:AST-090` FR1 (exits off by default) and FR10 |
There was a problem hiding this comment.
FR7 says it departs from the draft spec:AST-090 FR1 and FR10, but no AST-090 record exists in docs/specs at this head or on main, so an owner ruling on FR7 cannot read the rule it overrides. Cite the pull request that adds it, or drop the clause until it lands (the same reference recurs in Current-state impact).
| // The bar's title is hidden while the screen's large title shows, and | ||
| // fades in when the large title has scrolled under the bar. | ||
| const wasShownRef = useRef<boolean | null>(null); | ||
| useLayoutEffect(() => { |
There was a problem hiding this comment.
This new layout effect drives the compositor animator, but the comment only describes the outcome; react-component-runtime INV3 asks a new Effect to name the external system it synchronizes. Naming the animator it drives would cover it.
62e58c4 to
2578a94
Compare
There was a problem hiding this comment.
This stack needs an engineering owner's decision before it can be approved: no current spec governs NavigationStack's large titles, the draft spec:AST-094 it carries (including its proposed changes to the current theme-tokens, interaction-modality, buttons, user-states and AST-025 records) has no owner approval, and the new theme motion roles go beyond what the current AST-012 and theme-tokens records allow. An engineering owner can record that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, these still need fixing:
- The bar title shows at full opacity over a visible large title for about half a second when Done returns to a large-title root and when a scrolled large-title screen is pushed again, so clear a screen's collapsed flag when its card unmounts and ignore observer reports while a card is
content-visibility: hidden. - The
platformadaptation condition needs an amendment to AST-012 FR2's closed condition vocabulary, and AST-094 FR4, FR13 and DEC-1 need to describe the motion roles and the iOS idiom the stack ships, or the roles need to go. - After a pop that lands one or more levels deep, the screen underneath stays hidden with no bar, so a back swipe from there drags in an empty card.
- The diff against
mainis about 31,700 lines from 34 commits, so land or drop the predecessor pull requests first and let this one become its own large-title change.
[Automated review]
| {root: card, threshold: 0}, | ||
| ); | ||
| observer.observe(title); | ||
| return () => observer.disconnect(); |
There was a problem hiding this comment.
When this card unmounts its key stays in collapsedTitles, so pushing the same entry again mounts its bar with isLargeTitleShown false and the first-render set shows the bar title at full opacity over the visible large title until the new observer reports (react-component-runtime INV1). Clearing the flag here (onLargeTitleCollapse(model.key, false)) would fix it, since a newly mounted card starts at scroll 0.
| return undefined; | ||
| } | ||
| const observer = new IntersectionObserver( | ||
| ([entry]) => onLargeTitleCollapse(model.key, !entry.isIntersecting), |
There was a problem hiding this comment.
While a card is content-visibility: hidden (two levels down), this observer reports the title as not intersecting, so the root is marked collapsed; Done on About then remounts the root's bar with its title at opacity 1 over the large title. Ignoring entries while the card is hidden and keeping the last value would avoid the flash.
| : (view.models.at(-1)?.key ?? 'root'); | ||
| const underKey = | ||
| view.transition.kind !== 'none' | ||
| ? view.transition.under |
There was a problem hiding this comment.
After a pop finishes, transition.kind stays 'pop' and transition.under is the new top screen, so the screen actually underneath is isHidden with no bar layer; an iOS back swipe from there drags in a card whose content is skipped. Taking view.models.at(-2) once the transition has finished would fix it.
| * is everything else. Pair `ios` with `pointer: 'coarse'` for a touch | ||
| * profile, such as touch motion tuned per platform. | ||
| */ | ||
| platform?: 'ios' | 'other'; |
There was a problem hiding this comment.
spec:AST-012 FR2 lists width, pointer, contrast and motion and says "Future environment fields require explicit additions to this closed vocabulary"; this adds platform without that amendment. Carry the FR2 amendment for owner approval with the change, or drop the condition from this stack.
| measures (0.55 for a scroll view and a sheet), and a release MUST pick its | ||
| rest point by projecting its velocity the way its UIKit counterpart does: | ||
| with a deceleration rate, or with a measured projection time. | ||
| - **FR4 — Physics are not themable in touch presentations.** A touch |
There was a problem hiding this comment.
FR4 (and DEC-1's rejected option) says touch physics are not themable, but this stack lets every theme retune touch springs, durations and curves through defineTheme({motion}), and its touch-presentation resolver defines the theme's duration and easing tokens for touch outside the iOS idiom. An owner approving this record would approve a contract the code breaks; amend FR4, FR13 and DEC-1 or drop the motion roles.
2578a94 to
8f7b3c5
Compare
There was a problem hiding this comment.
This stack needs an engineering owner's decision before it can be approved: no current spec governs NavigationStack's large titles, the draft spec:AST-094 it carries (including its proposed changes to the current theme-tokens, interaction-modality, buttons, user-states and AST-025 records) has no owner approval, and the new theme motion roles and platform condition go beyond what the current AST-012 and theme-tokens records allow. An engineering owner can record that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, these still need fixing:
- The bar title shows at full opacity over a visible large title for about half a second when a scrolled large-title screen is pushed again and when Done returns to a large-title root, so clear a screen's collapsed flag when its card unmounts and ignore observer reports while a card is
content-visibility: hidden. - The
platformadaptation condition needs an amendment to AST-012 FR2's closed condition vocabulary, and AST-094 FR4, FR13 and DEC-1 need to describe the motion roles and the iOS idiom the stack ships, or the roles need to go. - After a pop that lands one or more levels deep, the screen underneath stays hidden with no bar, so a back swipe from there drags in an empty card.
- The diff against
mainis about 31,700 lines from 34 commits, so land or drop the predecessor pull requests first and let this one become its own large-title change.
[Automated review]
8f7b3c5 to
c8885c3
Compare
There was a problem hiding this comment.
This stack needs an engineering owner's decision before it can be approved: no current spec governs NavigationStack's large titles, the draft spec:AST-094 it carries (including its proposed changes to the current theme-tokens, interaction-modality, buttons, user-states and AST-025 records) has no owner approval, and the new defineTheme motion roles go beyond what the current theme-tokens record allows and what AST-094's own FR4 says. An engineering owner can record that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, these still need fixing:
- Done back to a scrolled large-title root brings it back scrolled 17 px less, with its bar title fading in from transparent, because the card's scroll listener also runs while the card is
content-visibility: hidden; skip it while hidden and keep the scroll offset. - The back swipe averages its last three velocities and keeps them through a still pause, while AST-100 FR2 requires the one tracker's release velocity, which is 0 after a still pause.
- The row press layer's
::beforeapplies on every pointer and hides callers'xstyle::beforedecorations for mouse users, which AST-017 FR59 rules out for touch work; scope it to a coarse pointer. - The diff against
mainis about 42,700 authored lines from 119 commits, so land or drop the predecessor pull requests first and let this one become its own large-title change.
[Automated review]
| const read = () => | ||
| onLargeTitleCollapse( | ||
| model.key, | ||
| title.offsetHeight > 0 && | ||
| card.scrollTop >= title.offsetTop + title.offsetHeight, | ||
| ); | ||
| read(); | ||
| card.addEventListener('scroll', read, {passive: true}); |
There was a problem hiding this comment.
This read also runs while the card is content-visibility: hidden two levels down, where Chromium reports scrollTop 0, so a scrolled root loses its collapse while hidden: Done back to a root scrolled 80 px returns it at 63 px, with its bar title starting transparent and under half opacity for 11 frames (react-component-runtime INV1). Skipping read while the card is hidden (an isHidden ref) and keeping the card's scroll offset across hiding would fix it.
| // UIKit's release velocity: the mean of the pan's last three | ||
| // velocities, the lift's own update included. A pause keeps it. | ||
| let v = 0; | ||
| if (recognizer.state === 'ended') { | ||
| swipe.velocities.push( | ||
| popVelocity(recognizer.velocity().x, width, direction), | ||
| ); | ||
| const recent = swipe.velocities.slice( | ||
| -navigation.interactivePopVelocitySamples, | ||
| ); | ||
| v = recent.reduce((sum, value) => sum + value, 0) / recent.length; |
There was a problem hiding this comment.
spec:AST-100 (current) names a navigation pop as a dragged surface and requires its release velocity to be the tracker's, 0 after a still pause, with no second estimate (FR2, FR3). Averaging the last three velocities keeps the speed through a pause, so a held swipe can still finish the pop; comparing recognizer.velocity().x at the lift would conform, or AST-100 needs an approved amendment for UIKit's averaging.
| '::before': { | ||
| content: '""', | ||
| position: 'absolute', | ||
| inset: 0, | ||
| zIndex: -1, | ||
| borderRadius: 'inherit', | ||
| pointerEvents: 'none', | ||
| backgroundColor: colorVars['--color-overlay-pressed'], | ||
| // On at once while the press is believed, and while a selection hold | ||
| // keeps the row selected (its own animations move it from there); | ||
| // nothing transitions it. | ||
| opacity: { | ||
| default: 0, | ||
| [ENABLED]: {default: null, [PRESSED_ON]: 1, [PRESSED_HELD]: 1}, | ||
| }, |
There was a problem hiding this comment.
These ::before declarations apply on every pointer, so an interactive row's own xstyle ::before is hidden at rest for mouse users too. AST-017 FR59 (current) lets touch work change fine-pointer behavior only to fix a fine-pointer defect or for named cross-device consistency, with before-and-after evidence; scoping them to COARSE would leave fine-pointer rows as they were.
c8885c3 to
f7f0f2d
Compare
…s, and the action-group tracker writes its own The header and writePress called it the one writer of the press attribute, but main's action-group tracker writes its groups' presses itself. They now say what holds, as meta#8076's gestures README does.
…he press attribute The release note said every press painter writes data-astryx-press through one internal writer, but the action-group tracker (hooks/actionGroupTracker.ts) writes its groups' presses itself, as the gestures README and utils/pressFeedback.ts already say. The note now says paintPress writes every press it tracks and the action-group tracker writes its own.
…llowance under every primary pointer The useMenuPress record's FR9 puts the trigger's held finger under spec:AST-094 FR29 under every primary pointer (FR43), so a finger on a touchscreen laptop may drift 13 pt before the open, as on a phone. The trigger read (pointer: coarse) as its press started and took the long press's 10 px where the pointer was fine. Three cases fail until it does not: a 12 pt drift opens under a fine pointer too, a (12, 12) settle opens there, and nothing reads a pointer query, the trigger's press included.
…lowance under every primary pointer The useMenuPress record's FR9 governs the trigger's held finger by spec:AST-094 FR29 under every primary pointer (FR43). The trigger now hands the long press FR29's 13 px drift allowance whatever the primary pointer, so it no longer reads (pointer: coarse) as its press starts, and the menus drop their coarsePointer import. ContextMenu keeps the long press's own allowable movement. The changeset says the same.
Strict ESLint reported six unused bindings in files this pull request carries: the host's HAPTIC_SWITCH_ATTRIBUTE import, two unused imports in the pan's tests (scrollProbe, ScreenEdgePanGestureRecognizer), the swipe test's drift, the presented-hit-testing test's gestures, and a GestureHost test's before, which now asserts what it was read for: a second finger's moves leave the pan where it was.
…eal angular speed The release velocity of a rotation whose listener sets rotation = 0 on each change, at 2 degrees every 16 ms, read 4.363 rad/s against the 2.182 rad/s of the same stream without a re-base: the test asserted only more than 1.5. It now asserts the 2.18 rad/s (spec:AST-100 FR2), and fails.
The rotation's velocity tracker samples the raw unwrapped angle, which setting rotation does not move, so the setter's shift of the samples read each re-base as extra turn and doubled the release velocity of a listener that re-bases on every change. The setter no longer touches the tracker.
…mates while it runs The proof read the element's animations 230 ms in, after the dip had ended, and asserted only that every animated key is opacity, which an empty list passes: with the cross-fade removed from the plan (the move still landing), the proof still passed. It now reads the keyframe keys right after the call starts and asserts they include opacity and nothing else; the same mutant fails it. Chromium and WebKit pass. (F1's in-flight read from c648034.)
…roller, and its claimed moves are cancelled under pointer events Two comparisons stop at the shadow boundary, each failing a case: - once the host's listener on a recognizer's inner node has seen the finger, isFingerDownIn reads it as outside a light-DOM scroller around that root, so a scroll under it is not the finger's (FR20); - under inputMode 'pointer-events', a claimed pan on a shadow host cancels none of its finger's moves over the inner content, since the finger's target is the inner node and its touches reach the host retargeted (FR19).
…the flat tree isFingerDownIn and the pointer-events finger match compare with containsAlongFlatTree, as the scroll watch already does: a finger on content inside an open shadow root is down in the scroller around that root, and its touches, retargeted to the shadow host, match its pointer finger, so a claimed pan cancels its moves there (spec:AST-094 FR19, FR20).
…achments asks, and only then The host let the last attachment decide whether an element is an action group (useGestureRecognizer always passes the option), and never recomputed it at a detach: a plain attachment after the group's turned the group off, and a group's attachment released left a recognizer that never asked taking presses under a fine pointer. Fails until the host counts the attachments that ask.
…ents asks The host counts the attachments that pass actionGroup: true and keeps the element a group while one remains, recomputed at each detach. An attachment that passes false, as useGestureRecognizer does by default, no longer turns a group off, and a group's last asking attachment taking its leave ends the group.
… that content The presented hit test takes the document's elementFromPoint, which answers a shadow host for anything inside its open root, and compares with Node.contains, which stops at the boundary: a recognizer on a control inside a moving sheet's shadow root gets no finger, and the tap's click goes to the host. Fails until the hit test descends into open roots and compares along the flat tree.
Where a moving element is drawn away from its layout box, the host hit-tests what is drawn under the finger with the document's elementFromPoint, which answers a shadow host for anything inside its open root. The hit test now descends through each open root's own elementFromPoint and compares with the movers along the flat tree, so a recognizer on a control inside a moving sheet's shadow root gets its finger and the tap clicks that control.
…nt lets go The host's touch-action rules sit in an adopted sheet or a style element per document or shadow root, and were never removed, so a page whose last recognizer detached kept them, against spec:AST-094 FR24 (nothing touch-related remains after the last instance unmounts). Fails until a root's rules go with its last marked element.
…th its last marked element The touch-action writer records which elements it marked in each document or shadow root, and when the last one is unmarked (its last recognizer detached) it removes that root's adopted sheet or style element, so nothing touch-related remains after the last instance unmounts (spec:AST-094 FR24). A later mark writes the rules again; an element marked in another root first lets go of the old one.
… the inline style it reads - The motion registry, the host's engine gate and the README name the presented hit test as an engine workaround (spec:AST-013 FR6) with its review condition (re-measure on each iOS release; remove it once iOS WebKit hit-tests a compositor-moved element where it is drawn) and its regression fixture. - While a curve runs the animator holds the value it starts from inline, so presentation.ts and the README no longer say the inline style holds the end.
…core/internal/touch Core ships the internal entry @astryxdesign/core/internal/touch, which re-exports the gesture primitives for Astryx's own packages, and the Lab-import guard fails a Lab import of core outside core's exports (architecture:react-component-runtime INV13). The README and the barrel's header named internal workspace imports and said nothing here is exported; they now name that entry.
…ill needs the element, and the listeners that cancel The release note, the stream's header, the host's comment and the README said the stream is one passive listener per event type and that a document-capture stopPropagation no longer starves the recognizers. Such a stop never hid a listener on the same node, and some subscriptions cancel. They now say what changed: an app's window-capture stopPropagation, or a document-capture stopImmediatePropagation registered before core, no longer hides a touch; a recognizer that must claim still needs the touch to reach its element; the gesture host's touchend, click and contextmenu cancel while a claimer is attached and the page hold's touchmove while it holds; and the stream stays ahead of an app's window-capture listener only until a type's listener is re-added. The README's list of blocking touchmove listeners gains the claimed touch's target and the document while a claiming element moves.
…default it moves
The {stylexOptions} form of astryxStylex now passes StyleX
treeshakeCompensation: true unless the caller sets it. spec:AST-017 FR3
asks the release note to name the old and new default and the option
that restores the old one; it now does.
…er and for the two stops the input stream fixes - A still finger with sub-pixel jitter over a scroll view keeps its tap and scrolls nothing (spec:AST-094 FR18–FR22). - A tap is still recognized under an app's window-capture stopPropagation, and under a document-capture stopImmediatePropagation registered before core's: the recognizers hear touches on the window's capture phase. Chromium and WebKit pass.
…s once Where nothing that claims is attached, the host's touchend listener is passive and cannot cancel the engine's own click after a retargeted tap clicks what is drawn. The shared click guard swallows that late click whatever is attached; this case pins it, as asked on meta#8293.
…esture host's subscriptions The gesture host subscribes no click: the one cancelling click subscriber is the click guard's, added at a window's first arm and kept. The release note, the stream's header and the gestures README listed click with the host's touchend and contextmenu; they now name the guard's own.
There was a problem hiding this comment.
Approval needs an engineering owner to provide or identify the governing spec for two additions that have none: NavigationStack with its large titles in @astryxdesign/lab, and the new stable isRtlElement export from @astryxdesign/core/hooks. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Whatever that decision, these need changes:
- A back swipe released while moving back can still pop (
spec:AST-094FR37), and its release velocity is a three-sample mean that survives a pause (spec:AST-100FR2), which a browser test currently expects. - The gesture host and both back swipes stay off under a mouse, trackpad or pen, while
spec:AST-094FR19 and FR43 attach gestures for every pointer except the screen-edge pan. - A large-title screen observes its
h1instead of its content for overflow, so containment can outlive the overflow (spec:AST-025FR14), and theisInteractivePopEnableddocs call the swipe iOS-only while it runs on any touch device. paintPressand theheldpaint have no caller, and at 32,052 authored lines the diff should shrink to the 570-line large-title change once #8108 lands.
[Automated review]
| * press controller: a group that tracks its selection, a row that keeps its | ||
| * selection through a navigation. | ||
| */ | ||
| export function paintPress( |
There was a problem hiding this comment.
Nothing calls paintPress or readPressPaint at this head, and nothing writes or styles the held paint, yet every Core pressable loads this module. Remove them until the pull request that uses them, and correct .changeset/gesture-press-tracking.md, which says paintPress writes the attribute "for every press it tracks".
Depends on #8108. Review only this PR's change.
User need
What led here: we are bringing Astryx's touch components to iOS quality on the mobile web. iOS top-level screens use a large title that hands over to the bar's small title as you scroll, and the stacked search field (#8181) sits under it.
Problem and solution fit
NavigationScreengainstitleDisplayMode: 'large' | 'inline'(defaultinline). The large title sits inside the scroll container, so it scrolls 1:1 with the rows. Once it is under the bar, the bar's title fades in and rises 15 px on a measured spring; scrolling back reverses it. Pushes, pops and back swipes carry each large title with its screen. While the large title shows, the bar's copy is hidden from assistive technology, and a push focuses the large title.Current specification
Proposed, for the owner's review. New measured constant:
navigation.inlineTitleRise(15), with an[experimental]core changeset.Public delta and caller burden
One optional prop; callers set
largeon top-level screens.Design and alternatives
Evidence
iOS 26.2 simulator: scroll hands over and back, push and pop, back swipe, a 600 ms block inside the handover. Chromium: Reduce Motion. Android 15 emulator: scroll.
Scope
Testing
In Chrome's device toolbar (a phone, touch), "Lab/NavigationStack › Large title":
What I ran: the unit tests and the browser proofs (Chromium and WebKit) passed. Two review findings have tests that failed before their fixes: popping back two levels shows the large title without the bar's, and a large-title screen pushed again starts large. The busy handover proof counts frames at the screen recording's own pace, which a loaded CI machine slows.
This now sits directly on #8108: #8114 closed after Cindy decided a row's selection is the product's call through
isSelected, and its change and #8082's left this branch. Recorded on the iOS 26.2 simulator and the Android 15 emulator; a real-device check is the release gate. Includes #8562 (Core's internal entry points); if those land differently, this follows.