Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions surface/web/src/components/ClampedBlock.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// @vitest-environment jsdom

import { act, cleanup, fireEvent, render, screen } from '@testing-library/react';
import { useEffect } from 'react';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { ClampedBlock } from './ClampedBlock';

Expand Down Expand Up @@ -118,3 +119,54 @@ describe('ClampedBlock', () => {
expect(observers).toHaveLength(1);
});
});

describe('ClampedBlock boundary identity', () => {
it('does not remount clamped content when the clamp re-renders', () => {
let mounts = 0;
function Child() {
useEffect(() => {
mounts += 1;
}, []);
return <span>Child</span>;
}

const { rerender } = render(
<ClampedBlock collapsedClassName="test-clamp" expandLabel="Show more ↓" collapseLabel="Show less ↑">
<Child />
</ClampedBlock>,
);
expect(mounts).toBe(1);

rerender(
<ClampedBlock collapsedClassName="test-clamp" expandLabel="Show all ↓" collapseLabel="Show less ↑">
<Child />
</ClampedBlock>,
);

expect(mounts).toBe(1);
});
});

describe('clamp containing blocks', () => {
// #544: `overflow: hidden` only clips descendants whose containing block runs
// through the clamping box. A `position: absolute` descendant (Tailwind's
// `sr-only`, which GFM footnotes and CriticMarkupView both emit) otherwise
// anchors to a positioned ancestor ABOVE the clamp, escapes the clip, and
// drags the scroll height into thousands of px of blank space. Every clamp in
// the app that can hold rendered markup must therefore be `relative`.
it('every collapsed clamp class in the app declares position: relative', async () => {
const sources = import.meta.glob('/src/**/*.tsx', { as: 'raw', eager: true }) as Record<string, string>;
const offenders: string[] = [];
for (const [path, source] of Object.entries(sources)) {
if (path.includes('ClampedBlock')) continue;
// Matches both the primitive's prop and hand-rolled clamp class strings.
const clampClasses = source.match(/(?:collapsedClassName=")([^"]*)"/g) ?? [];
for (const match of clampClasses) {
const value = match.slice('collapsedClassName="'.length, -1);
const clips = value.includes('overflow-hidden') || value.includes('line-clamp-');
if (clips && !value.split(/\s+/).includes('relative')) offenders.push(`${path}: ${value}`);
}
}
expect(offenders).toEqual([]);
});
});
119 changes: 86 additions & 33 deletions surface/web/src/components/ClampedBlock.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { createContext, useContext, useLayoutEffect, useRef, useState, type ReactNode } from 'react';
import {
createContext,
useCallback,
useContext,
useLayoutEffect,
useState,
type ReactNode,
type RefCallback,
} from 'react';

const ClampContext = createContext(false);

Expand All @@ -7,6 +15,79 @@ function classes(...values: Array<string | undefined | false>): string | undefin
return value || undefined;
}

export interface Clamp {
/** Attach to the element that carries `collapsedClassName`. */
contentRef: RefCallback<HTMLDivElement>;
/** True while the content is actually being clipped. */
clamped: boolean;
expanded: boolean;
/** Only true once the content is measured to exceed the clamp. */
overflows: boolean;
toggle: () => void;
/** Nesting guard for children — see ClampedBlock. */
ClampBoundary: ({ children }: { children: ReactNode }) => ReactNode;
}

/**
* The measuring half of ClampedBlock, for callers that must place the toggle
* themselves (EntryQuoteCard puts it in an action row beside Apply). Prefer
* ClampedBlock when the toggle can simply follow the content.
*/
export function useClamp(enabled = true): Clamp {
const insideClamp = useContext(ClampContext);
const canClamp = enabled && !insideClamp;
// The node is state, not a ref, so callers may mount the clamped element on a
// later render than the hook — EntryQuoteCard renders it only once its markup
// fetch resolves. A ref would attach silently, leaving this effect keyed on
// deps that never change, so the content was never measured at all.
const [element, setElement] = useState<HTMLDivElement | null>(null);
const [expanded, setExpanded] = useState(false);
const [overflows, setOverflows] = useState(false);

// Only measure while clamped. Expanded content does not overflow, so measuring
// it would incorrectly remove the control that lets the user collapse it.
useLayoutEffect(() => {
if (!element || !canClamp || expanded) return;

const measure = () => setOverflows(element.scrollHeight > element.clientHeight + 1);
measure();

// jsdom and older browser shells do not provide ResizeObserver.
if (typeof ResizeObserver === 'undefined') return;
const observer = new ResizeObserver(measure);
observer.observe(element);
return () => observer.disconnect();
}, [canClamp, element, expanded]);

const nested = insideClamp || canClamp;
// Memoized because React keys elements by component identity: a boundary
// rebuilt each render would be a new type every time, remounting everything
// clamped inside it (a whole message body) on every render.
const ClampBoundary = useCallback(
({ children }: { children: ReactNode }) => <ClampContext.Provider value={nested}>{children}</ClampContext.Provider>,
[nested],
);

return {
contentRef: setElement,
clamped: canClamp && !expanded,
expanded,
overflows: canClamp && overflows,
toggle: () => setExpanded((value) => !value),
ClampBoundary,
};
}

/**
* A block that clips its content and offers a toggle only once it measurably
* overflows. The clamp is published on context, so a ClampedBlock rendered
* inside another one declines to clamp: nesting two clips made the outer one
* bite only after the inner was released, so "Show more" SHRANK the text.
*
* `collapsedClassName` must carry `relative` whenever the content can hold an
* absolutely positioned descendant — `overflow: hidden` only clips descendants
* whose containing block runs through this box.
*/
export function ClampedBlock({
children,
collapsedClassName,
Expand Down Expand Up @@ -37,30 +118,7 @@ export function ClampedBlock({
overflowingClassName?: string;
toggleClassName?: string;
}) {
const insideClamp = useContext(ClampContext);
const canClamp = enabled && !insideClamp;
const contentRef = useRef<HTMLDivElement>(null);
const [expanded, setExpanded] = useState(false);
const [overflows, setOverflows] = useState(false);

// Only measure while clamped. Expanded content does not overflow, so measuring
// it would incorrectly remove the control that lets the user collapse it.
useLayoutEffect(() => {
const element = contentRef.current;
if (!element || !canClamp || expanded) return;

const measure = () => setOverflows(element.scrollHeight > element.clientHeight + 1);
measure();

// jsdom and older browser shells do not provide ResizeObserver.
if (typeof ResizeObserver === 'undefined') return;
const observer = new ResizeObserver(measure);
observer.observe(element);
return () => observer.disconnect();
}, [canClamp, expanded]);

const clamped = canClamp && !expanded;
const showToggle = canClamp && overflows;
const { contentRef, clamped, expanded, overflows, toggle, ClampBoundary } = useClamp(enabled);

return (
<>
Expand All @@ -72,15 +130,10 @@ export function ClampedBlock({
clamped && overflows && overflowingClassName,
)}
>
<ClampContext.Provider value={insideClamp || canClamp}>{children}</ClampContext.Provider>
<ClampBoundary>{children}</ClampBoundary>
</div>
{showToggle ? (
<button
type="button"
aria-expanded={expanded}
onClick={() => setExpanded((value) => !value)}
className={toggleClassName}
>
{overflows ? (
<button type="button" aria-expanded={expanded} onClick={toggle} className={toggleClassName}>
{expanded ? collapseLabel : expandLabel}
</button>
) : null}
Expand Down
70 changes: 60 additions & 10 deletions surface/web/src/components/EntryQuoteCard.test.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,38 @@
// @vitest-environment jsdom

import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { EntryInlineChip, EntryQuoteCard } from './EntryQuoteCard';
import { clearEntryResolveCacheForTests, type ResolvedEntryQuote } from '../lib/entryLinks';

const resolveEntryMock = vi.hoisted(() => vi.fn());

const observers: MockResizeObserver[] = [];

// The clamp measures once on mount and then re-measures through a ResizeObserver,
// which jsdom does not implement. Markup arrives after the mount measurement, so
// growing the card only reaches the clamp through the observer.
class MockResizeObserver {
constructor(private readonly callback: ResizeObserverCallback) {
observers.push(this);
}

observe(_target: Element) {}

disconnect() {}

trigger() {
this.callback([], this as unknown as ResizeObserver);
}
}

function setMetrics(element: Element, scrollHeight: number, clientHeight: number) {
Object.defineProperties(element, {
scrollHeight: { configurable: true, value: scrollHeight },
clientHeight: { configurable: true, value: clientHeight },
});
}

vi.mock('../api', () => ({
api: {
resolveEntry: resolveEntryMock,
Expand Down Expand Up @@ -37,6 +63,8 @@ function entry(overrides: Partial<ResolvedEntryQuote> = {}): ResolvedEntryQuote
beforeEach(() => {
clearEntryResolveCacheForTests();
resolveEntryMock.mockReset();
observers.length = 0;
vi.stubGlobal('ResizeObserver', MockResizeObserver);
});

afterEach(() => {
Expand Down Expand Up @@ -103,17 +131,16 @@ describe('entry links', () => {
expect(screen.getByRole('link').getAttribute('href')).toBe('/e/evt_404');
});

it('upgrades small markup artifacts into a tracked-changes card with clamp and expand', async () => {
const headers = new Headers({
'Content-Type': 'text/markdown; charset=utf-8',
'Content-Length': '180',
'X-Artifact-Seq': '4',
});
function renderMarkupCard() {
vi.stubGlobal(
'fetch',
vi.fn().mockResolvedValue({
ok: true,
headers,
headers: new Headers({
'Content-Type': 'text/markdown; charset=utf-8',
'Content-Length': '180',
'X-Artifact-Seq': '4',
}),
text: vi
.fn()
.mockResolvedValue(
Expand All @@ -130,7 +157,7 @@ describe('entry links', () => {
}),
);

render(
return render(
<EntryQuoteCard
entry={entry({
handle: 'art_00000000-0000-0000-0000-000000000001',
Expand All @@ -143,16 +170,39 @@ describe('entry links', () => {
})}
/>,
);
}

it('upgrades small markup artifacts into a tracked-changes card', async () => {
renderMarkupCard();

expect(screen.getAllByText('memo.md').length).toBeGreaterThan(0);
expect(await screen.findByText('Edited memo')).toBeTruthy();
expect(screen.getByText('markup')).toBeTruthy();
expect(screen.getByText('Show all changes (2)')).toBeTruthy();
expect(screen.getByText('old').className).toContain('atrium-critic-view-del');
expect(screen.getByText('new').className).toContain('atrium-critic-view-ins');
// The toggle is measured, not assumed: a diff this short leaves nothing
// hidden, so neither a control nor a fade is offered for it.
expect(screen.queryByRole('button', { name: /Show all changes/ })).toBeNull();
expect(screen.queryByTestId('markup-clamp-fade')).toBeNull();
});

it('measures the clamp even though the markup mounts after the fetch resolves', async () => {
renderMarkupCard();
await screen.findByText('Edited memo');

const clamped = screen.getByText('old').closest('.overflow-hidden');
expect(clamped).toBeTruthy();
setMetrics(clamped!, 600, 314);
act(() => {
for (const observer of observers) observer.trigger();
});

expect(screen.getByTestId('markup-clamp-fade')).toBeTruthy();
fireEvent.click(screen.getByRole('button', { name: 'Show all changes (2)' }));
expect(screen.getByRole('button', { name: 'Show fewer changes' })).toBeTruthy();
// Expanding releases the clip and the fade, so the whole diff is reachable.
expect(screen.getByText('old').closest('.overflow-hidden')).toBeNull();
expect(screen.queryByTestId('markup-clamp-fade')).toBeNull();
});

it('leaves non-markup artifact cards on the excerpt renderer', async () => {
Expand Down
Loading
Loading