diff --git a/apps/web/app/space/[id]/(space)/layout.tsx b/apps/web/app/space/[id]/(space)/layout.tsx index 7367be1d15..4f1254f831 100644 --- a/apps/web/app/space/[id]/(space)/layout.tsx +++ b/apps/web/app/space/[id]/(space)/layout.tsx @@ -37,6 +37,7 @@ import { SpaceMembers } from '~/partials/space-page/space-members'; import { SpacePageMetadataHeader } from '~/partials/space-page/space-metadata-header'; import { SpaceTabs } from '~/partials/space-page/space-tabs'; import type { PersonRecordCounts } from '~/partials/space-page/space-tabs'; +import { SPACE_TABS_ANCHOR } from '~/partials/space-page/space-tabs-anchor'; import { cachedFetchEntitiesBatch, cachedFetchEntityPage } from '../../(entity)/[id]/[entityId]/cached-fetch-entity'; import { cachedFetchSpace } from '../cached-fetch-space'; @@ -209,17 +210,28 @@ export default async function Layout(props0: LayoutProps) { ) : null} - - - + {/* + * The tab bar is a link target, so a link can send the reader to the tabs rather than + * to the top of the page — see `withSpaceTabsAnchor`. + * + * The margin clears the navbar, which is `sticky top-0 h-11` and would otherwise cover + * the row the link exists to show — the same 44px the hub panel and the side rail sit + * below as `top-11`. `scroll-mt-14` is that plus 12px, so the tabs land under the + * navbar rather than welded to it. + */} +
+ + + +
diff --git a/apps/web/core/debates/browse/debate-feed-player.tsx b/apps/web/core/debates/browse/debate-feed-player.tsx index 9b0865098f..f177781c45 100644 --- a/apps/web/core/debates/browse/debate-feed-player.tsx +++ b/apps/web/core/debates/browse/debate-feed-player.tsx @@ -392,35 +392,49 @@ function DebaterVideo({ )} - {/* Debater identity: avatar + name + position, opens their personal space in the side panel. */} - + {participant && ( - - {participant.position_label} - + votes.castVote(participant)} + /> )} - - - {participant && ( - votes.castVote(participant)} - /> - )} + {scrubber &&
{scrubber}
} diff --git a/apps/web/partials/profile/profile-activity-section.test.tsx b/apps/web/partials/profile/profile-activity-section.test.tsx index b5d2342a98..c94bcb53fa 100644 --- a/apps/web/partials/profile/profile-activity-section.test.tsx +++ b/apps/web/partials/profile/profile-activity-section.test.tsx @@ -1,5 +1,5 @@ import '@testing-library/jest-dom/vitest'; -import { cleanup, render, screen } from '@testing-library/react'; +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; import * as React from 'react'; @@ -47,6 +47,124 @@ const kind = (over: Partial[ ...over, }); +const rect = (width: number, height: number): DOMRect => ({ + x: 0, + y: 0, + top: 0, + right: width, + bottom: height, + left: 0, + width, + height, + toJSON: () => ({}), +}); + +/** + * A 390×600 mobile viewport sitting 400px down a synthetic profile page. + * + * Includes a `ResizeObserver`, which JSDOM has none of. Without one the hook takes its + * no-observer path and never attaches the scroll listener — so anything asserted about scrolling + * passed for the wrong reason, whatever the code did. + */ +function mockMobileActivityGeometry(pageHeightWithoutActivity: number) { + let notifyResize: (() => void) | null = null; + class TestResizeObserver { + constructor(callback: ResizeObserverCallback) { + notifyResize = () => callback([], this as unknown as ResizeObserver); + } + observe() {} + unobserve() {} + disconnect() {} + } + vi.stubGlobal('ResizeObserver', TestResizeObserver); + + const sectionHeight = { debates: 500, claims: 250 }; + + const originalRect = HTMLElement.prototype.getBoundingClientRect; + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function (this: HTMLElement) { + if ('activitySection' in this.dataset) { + const debatesSelected = + this.querySelector('button[aria-pressed="true"]')?.textContent?.includes('Debates'); + return rect(390, debatesSelected ? sectionHeight.debates : sectionHeight.claims); + } + + if ('activityScrollReserve' in this.dataset) { + return rect(390, Number.parseFloat(this.style.height) || 0); + } + + return originalRect.call(this); + }); + + const scroll: { y: number; moveAfterFirstRead?: number } = { y: 400 }; + let reads = 0; + vi.spyOn(window, 'scrollY', 'get').mockImplementation(() => { + reads += 1; + if (scroll.moveAfterFirstRead !== undefined && reads > 1) return scroll.moveAfterFirstRead; + return scroll.y; + }); + const viewport = { height: 600 }; + vi.spyOn(window, 'innerHeight', 'get').mockImplementation(() => viewport.height); + + const page = { withoutActivity: pageHeightWithoutActivity }; + vi.spyOn(document.documentElement, 'scrollHeight', 'get').mockImplementation(() => { + const section = document.querySelector('[data-activity-section]'); + const reserve = document.querySelector('[data-activity-scroll-reserve]'); + return ( + page.withoutActivity + + (section?.getBoundingClientRect().height ?? 0) + + (reserve?.getBoundingClientRect().height ?? 0) + ); + }); + + // Moves the mocked position, the way a real one does. Mocked as a no-op it silently turned + // every "and then the reader is back at 400" into a page still sitting where it was, which is + // how a viewport resize could lose the reader with the tests all passing. + const scrollTo = vi.spyOn(window, 'scrollTo').mockImplementation(((_x: number, y: number) => { + scroll.y = y; + }) as typeof window.scrollTo); + + return { + scrollTo, + // Stands in for the browser moving the reader: there is no real layout here to clamp a scroll + // position, and the recovery path exists for exactly that case. + scroll, + // And for the claim cards growing as their queries land, which is the other way the sizing + // path runs after a switch. Move `sectionHeight` first; the observer reads it. + sectionResized: () => notifyResize?.(), + sectionHeight, + // The viewport, which a phone changes on its own as its chrome collapses, and the rest of the + // page, which goes on loading after the switch. + viewport, + page, + }; +} + +/** + * The card with both kinds, and the reserve element the tests below assert on. + * + * Every reserve test wants the same two kinds — a tall Debates view and a short Claims one — since + * what they are about is the switch between them, not what is in either. + */ +function renderActivity() { + const { container } = render( + + ); + + return { reserve: container.querySelector('[data-activity-scroll-reserve]') }; +} + +/** + * Wait for the scroll listener to arm. + * + * It arms a frame after a swap, so the swap's own scroll events are not read as the reader moving — + * a test that scrolls straight after a switch is testing the disarmed frame and nothing else. + */ +function armScrollListener() { + return act(async () => { + await new Promise(resolve => requestAnimationFrame(() => resolve(null))); + }); +} + /** * What the Activity card says when half of it did not arrive (GEO-2859). * @@ -56,7 +174,10 @@ const kind = (over: Partial[ * no positions, while the rail beside it counted 208. */ describe('ProfileActivitySection', () => { - afterEach(cleanup); + afterEach(() => { + cleanup(); + vi.restoreAllMocks(); + }); it('renders nothing when both kinds are genuinely empty', () => { // Most accounts have never been in a debate; a heading over blank space @@ -102,4 +223,311 @@ describe('ProfileActivitySection', () => { expect(screen.getByRole('button', { name: /Debates/ })).toHaveTextContent('—'); }); + + it('sends See all to the tab bar rather than the top of the page', () => { + render( + + ); + + // Without the fragment the reader lands at the top of the profile — a screenful of cover, + // avatar, name, roles and bio — rather than on the list they clicked for. + expect(screen.getByRole('link', { name: /See all debates/ })).toHaveAttribute( + 'href', + '/space/s/debates#space-tabs' + ); + + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + + expect(screen.getByRole('link', { name: /See all claims/ })).toHaveAttribute( + 'href', + '/space/s/positions#space-tabs' + ); + }); + + it('reserves the lost mobile document height while switching between kinds', () => { + mockMobileActivityGeometry(600); + + const { reserve } = renderActivity(); + + expect(reserve).toHaveStyle({ height: '0px' }); + + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + expect(reserve).toHaveStyle({ height: '150px' }); + + fireEvent.click(screen.getByRole('button', { name: /Debates/ })); + expect(reserve).toHaveStyle({ height: '0px' }); + + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + expect(reserve).toHaveStyle({ height: '150px' }); + }); + + /** + * The reserve has to be *displayed* on the screens it exists for, which is the one thing the tests + * above could not see: they mock geometry, JSDOM applies no Tailwind, and so a reserve that the + * browser was hiding still measured and asserted perfectly while doing nothing on a real phone. + * + * The breakpoints here are desktop-first — `md` is `@media (max-width: 767px)`, see styles.css — so + * `md:hidden` hides an element *on mobile*. This element carried exactly that, which made the whole + * mechanism inert on the only viewports it was written for (GEO-2974). Asserting the class is + * crude, but it is the only trace of the mistake that survives into JSDOM. + */ + it('keeps the reserve displayed at the mobile breakpoint, where it is the only thing holding the page', () => { + mockMobileActivityGeometry(600); + + const { reserve } = renderActivity(); + + // `md:hidden` would switch it off below 768px, which is every phone. + expect(reserve?.className).not.toMatch(/(^|\s)md:hidden(\s|$)/); + // And it has to be on at that width rather than merely not off. + expect(reserve?.className).toMatch(/(^|\s)md:block(\s|$)/); + }); + + /** + * Susan Winter's profile, which is where this was reported: an Activity card with nothing below it, + * so the document barely exceeds the viewport and the card's own height is the entire scroll range. + * Switching to the shorter view takes more height out of the page than the page has to spare. + */ + it('holds the whole missing range on a profile with nothing below Activity', () => { + // 40px of page besides the card: a name and an avatar, no sections after it. + mockMobileActivityGeometry(40); + + const { reserve } = renderActivity(); + + // Nothing to hold before a switch: the page is however tall it is. + expect(reserve).toHaveStyle({ height: '0px' }); + + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + + // Claims: 40 + 250 = 290 natural, and the reader's viewport bottom is at 1000. + // That it is also allowed to render at this width is the test above. + expect(reserve).toHaveStyle({ height: '710px' }); + }); + + /** + * The reserve is in the document a frame before React swaps the view, but the gallery can paint + * empty while its queries land, and a document shorter than the reserve can cover takes the reader + * with it. Where that happens they are put back, before the browser paints. + */ + it('puts the reader back when a shrink beat the reserve to it', () => { + const { scroll, scrollTo } = mockMobileActivityGeometry(600); + + renderActivity(); + + // The switch reads the position once on the way in; by the time the effect looks again the + // browser has moved the reader, which is the ordering this recovery exists for. + scroll.moveAfterFirstRead = 150; + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + + expect(scrollTo).toHaveBeenCalledWith(0, 400); + }); + + /** + * The reader has to be able to scroll afterwards. Sizing the reserve and correcting the position + * were the same function once, and that function ran on every scroll — so scrolling down, which + * leaves the held position where it was, corrected the reader straight back to it. The page read + * as refusing to move (GEO-2974). + */ + it('lets the reader scroll down after a switch', async () => { + const { scroll, scrollTo } = mockMobileActivityGeometry(600); + + renderActivity(); + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + scrollTo.mockClear(); + + await armScrollListener(); + + scroll.y = 700; + fireEvent.scroll(window); + + expect(scrollTo).not.toHaveBeenCalled(); + }); + + it('stops holding height the reader has scrolled back above', async () => { + const { scroll } = mockMobileActivityGeometry(600); + const { reserve } = renderActivity(); + + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + expect(reserve).toHaveStyle({ height: '150px' }); + + await armScrollListener(); + + // Back up to the top: nothing below the viewport needs holding any more. + scroll.y = 0; + fireEvent.scroll(window); + + expect(reserve).toHaveStyle({ height: '0px' }); + }); + + /** + * A phone changes its own viewport height: the browser chrome collapses as the reader scrolls and + * comes back when they stop, with nothing on the page moving. The held height is measured against + * that viewport, so a taller one needs more below it — and before this was watched, the reader + * could be clamped upward by exactly the height of a hidden URL bar. + */ + it('re-sizes the reserve when the viewport height changes', () => { + const { viewport } = mockMobileActivityGeometry(600); + const { reserve } = renderActivity(); + + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + expect(reserve).toHaveStyle({ height: '150px' }); + + // 600 → 700 of viewport. Held at 400, the page now has to reach 1100 rather than 1000, against + // a natural 850. + viewport.height = 700; + fireEvent.resize(window); + + expect(reserve).toHaveStyle({ height: '250px' }); + }); + + /** + * The section is not the only thing on the page that moves after a switch. A cover image landing + * above Activity changes the document height without changing the section at all, so a natural + * height remembered from the switch is wrong — and wrong in the direction that leaves the reader + * scrolling into space the page no longer needs. + */ + it('measures the rest of the page rather than remembering it', () => { + const { page, sectionHeight, sectionResized } = mockMobileActivityGeometry(600); + const { reserve } = renderActivity(); + + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + expect(reserve).toHaveStyle({ height: '150px' }); + + // The cards grow, and the cover lands above them in the same frame. + page.withoutActivity = 700; + sectionHeight.claims = 300; + act(() => { + sectionResized(); + }); + + // 700 + 300 reaches the reader's 1000 exactly. Carried from the switch instead, the page above + // is still believed to be 600 and 100px would be held for no one. + expect(reserve).toHaveStyle({ height: '0px' }); + }); + + /** + * A growing viewport moves the reader before this hook hears about it, and by construction rather + * than by chance: while the reserve holds anything it sizes the document so `holdY` is exactly the + * furthest the page can scroll, so 100px more viewport is 100px less maximum, every time. Sizing + * the reserve back up returns the range but not the reader. + */ + it('puts the reader back when a growing viewport clamps them', () => { + const { scroll, viewport, scrollTo } = mockMobileActivityGeometry(600); + const { reserve } = renderActivity(); + + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + expect(reserve).toHaveStyle({ height: '150px' }); + scrollTo.mockClear(); + + // The URL bar hides. The document is 1000 tall, so the furthest it can scroll drops from 400 to + // 300 and the browser takes the reader with it before the resize handler runs. + viewport.height = 700; + scroll.y = 300; + fireEvent.resize(window); + + expect(reserve).toHaveStyle({ height: '250px' }); + expect(scrollTo).toHaveBeenCalledWith(0, 400); + + // And the clamp arrives as a scroll event afterwards. Read as the reader moving up it would + // lower the hold to 300 and shrink the reserve to 150, undoing the restore that just happened. + fireEvent.scroll(window); + expect(reserve).toHaveStyle({ height: '250px' }); + }); + + /** + * A viewport that shrinks comes back. On iOS the URL bar returning takes height away and hiding + * it again gives the height back, so a shrink that happens to need nothing held must not retire + * the swap — there would be nothing left to rebuild the reserve when the height returns, and the + * reader would be clamped by the difference. + */ + it('keeps holding across a viewport that shrinks and grows back', () => { + const { viewport } = mockMobileActivityGeometry(600); + const { reserve } = renderActivity(); + + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + expect(reserve).toHaveStyle({ height: '150px' }); + + // The URL bar comes back, far enough that 400 + 450 is exactly the natural 850 and nothing + // needs holding — which is a fact about this instant, not about the switch being over. + viewport.height = 450; + fireEvent.resize(window); + expect(reserve).toHaveStyle({ height: '0px' }); + + // It hides again. + viewport.height = 700; + fireEvent.resize(window); + expect(reserve).toHaveStyle({ height: '250px' }); + }); + + /** + * The swap is over once the page can hold the reader without help, and it has to actually end. + * Left armed, `holdY` outlives the switch it belongs to: a shrink long afterwards would size a + * reserve from a position the reader left, and hand them a screen of blank space to scroll into. + */ + it('stops holding once the page is tall enough on its own', () => { + const { sectionHeight, sectionResized } = mockMobileActivityGeometry(600); + const { reserve } = renderActivity(); + + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + expect(reserve).toHaveStyle({ height: '150px' }); + + // The claim cards finish loading, and the page is long enough on its own. + sectionHeight.claims = 500; + act(() => { + sectionResized(); + }); + expect(reserve).toHaveStyle({ height: '0px' }); + + // Whatever shrinks the section after that is not this switch's business. + sectionHeight.claims = 250; + act(() => { + sectionResized(); + }); + expect(reserve).toHaveStyle({ height: '0px' }); + }); + + /** + * The other caller of the sizing path. A claim card grows when its queries land, which resizes + * the section — and resizing the reserve is the whole response to that. Moving the reader is not, + * wherever they have got to by then: the correction belongs to the swap that asked for it. + */ + it('leaves the reader alone when the cards grow after a switch', async () => { + const { scroll, sectionResized, scrollTo } = mockMobileActivityGeometry(600); + + renderActivity(); + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + scrollTo.mockClear(); + await armScrollListener(); + + scroll.y = 700; + act(() => { + sectionResized(); + }); + + expect(scrollTo).not.toHaveBeenCalled(); + }); + + it('leaves the reader alone when the reserve did its job', () => { + const { scrollTo } = mockMobileActivityGeometry(600); + + renderActivity(); + + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + + expect(scrollTo).not.toHaveBeenCalled(); + }); + + it('adds no reserve when content below Activity already preserves the scroll range', () => { + mockMobileActivityGeometry(900); + + const { reserve } = renderActivity(); + + fireEvent.click(screen.getByRole('button', { name: /Claims/ })); + + expect(reserve).toHaveStyle({ height: '0px' }); + }); }); diff --git a/apps/web/partials/profile/profile-activity-section.tsx b/apps/web/partials/profile/profile-activity-section.tsx index dfe1241375..016f6ddf14 100644 --- a/apps/web/partials/profile/profile-activity-section.tsx +++ b/apps/web/partials/profile/profile-activity-section.tsx @@ -15,6 +15,7 @@ import { RightArrowLongSmall } from '~/design-system/icons/right-arrow-long-smal import { PrefetchLink as Link } from '~/design-system/prefetch-link'; import { ExploreFeedCard } from '~/partials/explore/explore-feed-card'; +import { withSpaceTabsAnchor } from '~/partials/space-page/space-tabs-anchor'; import { GalleryClaimCard } from './gallery-claim-card'; @@ -88,74 +89,301 @@ export function ProfileActivitySection({ kinds }: { kinds: ActivityKind[] }) { // cannot shift the selection out from under them. const selected = available.find(kind => kind.key === selectedKey) ?? available[0]; + const { sectionRef, reserveRef, prepareSwitch } = useMobileActivityHeightReserve(selected?.key); + // Nothing at all rather than an empty card. A heading over a blank space reads // as a page that failed to load, and most accounts have never been in a debate. if (kinds.some(kind => kind.isLoading) || available.length === 0 || !selected) return null; return ( -
- {/* The toggles sit to the right of the heading, and wrap below it rather +
+
+ {/* The toggles sit to the right of the heading, and wrap below it rather than squeezing into it on a narrow screen. */} -
-

Activity

+
+

Activity

- {/* Only when there is a choice to make. One pill on its own is a label + {/* Only when there is a choice to make. One pill on its own is a label dressed up as a control. */} - {available.length > 1 && ( -
- {available.map(kind => { - const isSelected = kind.key === selected.key; - - return ( - - ); - })} -
+ {available.length > 1 && ( +
+ {available.map(kind => { + const isSelected = kind.key === selected.key; + + return ( + + ); + })} +
+ )} +
+ + {selected.isError && selected.rows.length === 0 ? ( + /* + * No retry here on purpose. This card is a summary; the tab its count + * links to holds the authoritative list and offers the retry, so a + * second control here would be a second thing to keep in step. + */ +

Couldn’t load {selected.label.toLowerCase()}.

+ ) : ( + )} -
- - {selected.isError && selected.rows.length === 0 ? ( - /* - * No retry here on purpose. This card is a summary; the tab its count - * links to holds the authoritative list and offers the retry, so a - * second control here would be a second thing to keep in step. - */ -

Couldn’t load {selected.label.toLowerCase()}.

- ) : ( - - )} + {/* + * Lands on the tab bar, not the page top. + * + * A navigation lands at the top of the page, which on a phone is a screenful of cover, + * avatar, name, roles and bio — none of it what "See all debates" was clicked for. The + * fragment puts the tab row under the navbar instead, so the list opens at the top of the + * screen with the underlined tab above it saying where the reader has been sent. + */} + + {selected.seeAllLabel} + + +
- - {selected.seeAllLabel} - - -
+ {/* + * On narrow screens this supplies only the document height missing below + * the current viewport. The reserve sits outside the section, so Claims + * stays compact while switching away from the taller Debates view cannot + * clamp the viewport upward. Profiles with content below Activity need no + * reserve at all, and desktop keeps its natural layout. + * + * `hidden md:block`, not `md:hidden`: the breakpoints here are desktop-first + * (`md` is `max-width: 767px`, see styles.css), so `md:hidden` hid this on + * exactly the phones it exists for — `display: none` reserves no height, and + * the fix was inert on the only screens that needed it. + */} +
+
); } +/** + * Keep the reader where they were while Activity views of different heights are swapped. + * + * Switching to a shorter view makes the document shorter, and a document that no longer reaches the + * reader's viewport bottom has no scroll position to hold them at — the browser moves them up. On a + * profile with content below Activity there is other height to absorb that; on a short one, like a + * person with only an Activity card, there is none, and the page jumps to the top. + * + * Two things hold the position, in order of precedence: + * + * 1. A sibling reserve supplies exactly the document height missing below the viewport, so the + * shorter view never shortens the scrollable range. It is a sibling rather than a `min-height` + * on the section, which keeps a three-row Claims gallery from sitting inside a debate-sized box. + * 2. If the position is lost anyway — the gallery can paint empty for a frame before its queries + * land, which shortens the document below even the reserve's reach — it is put back before the + * browser paints. + */ +function useMobileActivityHeightReserve(selectedKey: string | undefined) { + const sectionRef = React.useRef(null); + const reserveRef = React.useRef(null); + const swapRef = React.useRef<{ + /** The layout this was calculated for. A different one invalidates it — see `sizeReserve`. */ + width: number; + /** The position being held for the reader, which only ever moves up. */ + holdY: number; + } | null>(null); + const prepareSwitch = React.useCallback(() => { + const section = sectionRef.current; + const reserve = reserveRef.current; + if (!section || !reserve) return; + + const { width, height } = section.getBoundingClientRect(); + swapRef.current = { width, holdY: window.scrollY }; + + // Hold the outgoing view's whole height before React replaces it. Waiting for the layout effect + // would leave a window where the document is short and the position is already gone; over- + // reserving now costs nothing, because the effect below replaces it with the exact figure before + // anything is painted. + reserve.style.height = `${height}px`; + }, []); + + React.useLayoutEffect(() => { + const section = sectionRef.current; + const reserve = reserveRef.current; + if (!section || !reserve) return; + + // Sizing only. Nothing here moves the reader: this runs on every scroll, and a function that + // both sizes the reserve and corrects the position will correct it every time they scroll — + // which reads as the page refusing to move (GEO-2974). + const sizeReserve = () => { + const { width } = section.getBoundingClientRect(); + const swap = swapRef.current; + + // A new layout width (rotation, resized side panel, breakpoint change) has different card + // wrapping. Drop the old calculation; the next tab switch will establish one for the new + // layout. + if (!swap || Math.abs(swap.width - width) > 1) { + swapRef.current = null; + reserve.style.height = '0px'; + return 0; + } + + // Measured now rather than carried from the switch. The page keeps moving afterwards — the + // cards grow as their queries land, a cover image arrives above, the rail settles — and a + // figure taken once is wrong for every one of those. Subtracting what the reserve is + // currently contributing is what makes this the height the page would have without it. + const naturalDocumentHeight = document.documentElement.scrollHeight - reserve.getBoundingClientRect().height; + const held = Math.max(0, swap.holdY + window.innerHeight - naturalDocumentHeight); + reserve.style.height = `${held}px`; + + return held; + }; + + /** + * Size, and let the swap go once the page no longer needs it. + * + * A swap that outlives its own settling is state waiting to be wrong: the cards grow, the + * reserve reaches zero, and `holdY` sits there for as long as the reader stays on this tab — + * so a shrink an hour later would conjure height back out of a position they left behind, and + * leave them scrolling into blank space. + * + * Zero is the safe moment to let go precisely because nothing is being held at it, so dropping + * the swap cannot move anybody. Not on the first sizing though — that one runs before the + * correction below, and the correction needs the swap it belongs to. + * + * Only for changes that do not come back. Cards growing and the reader scrolling up both leave + * the page needing less than it did, and go on needing less. A viewport is not like that: it + * shrinks and grows again as the URL bar returns and hides, so settling on a shrink would + * retire the swap during the half of that cycle where nothing is needed, and leave nothing to + * rebuild the reserve on the half where it is. Resizes size, and do not settle. + */ + const sizeAndSettle = () => { + if (sizeReserve() === 0) swapRef.current = null; + }; + + sizeReserve(); + + // The reserve is in the document now, so the position asked for is reachable again. If a frame + // painted before it was — the gallery can paint empty while its queries land — the reader is put + // back here, once, before the browser paints. Once, because this is a correction for the swap + // that just happened and not a rule about where the page may be scrolled to. + const swap = swapRef.current; + if (swap && Math.abs(window.scrollY - swap.holdY) > 1) { + window.scrollTo(0, swap.holdY); + } + + if (typeof ResizeObserver === 'undefined') return; + // Claim cards grow as their queries land. Shrink the reserve by the same amount so the overall + // document height stays steady rather than drifting. + const observer = new ResizeObserver(sizeAndSettle); + observer.observe(section); + + // Armed a frame late, so the scroll events belonging to the swap itself — the correction above, + // and any clamp it was correcting — are not read as the reader choosing to move. + let armed = false; + const arm = requestAnimationFrame(() => { + armed = true; + }); + + const onScroll = () => { + if (!armed) return; + + const current = swapRef.current; + if (!current) return; + + // Once the reader moves up of their own accord, stop holding space they no longer need — + // and back at the top there is nothing left to hold, so the swap goes with it. Moving down + // needs nothing held and nothing released. + if (window.scrollY < current.holdY) { + current.holdY = window.scrollY; + sizeAndSettle(); + } + }; + window.addEventListener('scroll', onScroll, { passive: true }); + + // `held` is measured against the viewport, and on a phone the viewport changes without anything + // else on the page moving: iOS Safari grows `innerHeight` when its URL bar collapses under a + // scroll and shrinks it back when the reader stops, firing `resize` both ways. A taller + // viewport needs more held below it, and nothing here was watching — so the reader could be + // clamped upward by exactly the height of a hidden URL bar. That is the device this was + // reported from; Chrome on Android pins its layout viewport to the largest size instead, so + // `innerHeight` never moves there and there is nothing to react to. + // + // `resize` rather than `visualViewport`, because `window.innerHeight` is the figure the sum + // above uses and the two do not always agree. And it sizes without settling: see + // `sizeAndSettle` for why a reversible change must not retire the swap. + // + // Sizing alone is not enough, because a growing viewport has already moved the reader by the + // time this runs. While the reserve holds anything, it sizes the document so that `holdY` is + // *exactly* the furthest the page can scroll — that is what holding the position means — so a + // viewport 100px taller drops the maximum by 100 and the browser takes the reader with it, + // every time rather than occasionally. Restoring afterwards is the same one-shot correction the + // swap itself gets, for the same reason. + const onViewportResize = () => { + const swap = swapRef.current; + if (!swap) { + sizeReserve(); + return; + } + + // Synchronously, inside the resize handler, and that ordering is the whole guard. The clamp + // also arrives as a scroll event, which is dispatched after this runs — so by the time + // `onScroll` reads the position it is the restored one, and there is nothing there for it to + // mistake for the reader moving up. + const target = swap.holdY; + sizeReserve(); + + if (Math.abs(window.scrollY - target) > 1) window.scrollTo(0, target); + }; + window.addEventListener('resize', onViewportResize); + + return () => { + cancelAnimationFrame(arm); + observer.disconnect(); + window.removeEventListener('scroll', onScroll); + window.removeEventListener('resize', onViewportResize); + }; + }, [selectedKey]); + + return { sectionRef, reserveRef, prepareSwitch }; +} + function ActivityGallery({ rows, responseByClaimId, @@ -181,25 +409,34 @@ function ActivityGallery({ // start. The gate names the one nearest the middle. {/* - * `snap-x` so a flick lands on a card rather than between two. + * The wrapper, not the scroller, carries both of these. + * + * `@container`, because the container types imply `contain: inline-size`, and containing the + * element whose overflow is the whole point is a bad trade for one class. The wrapper is also + * the width the reader actually sees, which is what the cards want to measure — `cqw` below + * reads this element, so widening only what scrolls would give the reader more to look at + * without giving the cards any more to size against. * - * The gap at either end is a spacer element rather than padding on the - * scroller: a scroll container's trailing padding is dropped by every - * browser that matters, so `p-4` gave 16px on the left and nothing on the - * right. Spacers are honoured on both sides, and `scroll-px` keeps a - * snapped card off the edge it lands against. + * The bleed takes it out through the app shell's own gutter on a phone, so a card can use the + * full width and the one behind it is cut off by the screen edge rather than by a panel. `2ch` + * is the shell's figure (`2xl:px-[2ch]` in `app/entry.tsx`) and the two have to stay equal, or + * the gallery hangs off the side of the document and every profile scrolls sideways. `md` is + * inside `2xl` in a desktop-first scale, so the gutter is always there to cancel. */} - {/* `@container` on a wrapper rather than on the scroller itself: the - container types imply `contain: inline-size`, and containing the - element whose overflow is the whole point is a bad trade for one class. - The wrapper is the width the reader actually sees, which is what the - cards want to measure — see `GalleryCard`. */} -
+
+ {/* + * `snap-x` so a flick lands on a card rather than between two. + * + * The gap at either end is a spacer element rather than padding on the scroller: a scroll + * container's trailing padding is dropped by every browser that matters, so `p-4` gave 16px + * on the left and nothing on the right. Spacers are honoured on both sides, and `scroll-px` + * keeps a snapped card off the edge it lands against. + */}
- + {shown.map(row => (