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
59 changes: 38 additions & 21 deletions plugins/thread-briefs/app.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -147,39 +147,56 @@ describe("the header popover", () => {
slot.lifecycle.unmount();
});

it("caps its height and scrolls, so a long brief is reachable on a phone", async () => {
const slot = await render({ isCompactViewport: true });
/**
* These assert the *inline style*, not class names. An earlier version
* checked `className` contained `w-[calc(100vw-1rem)]` and passed happily
* while the panel rendered unconstrained: the class name being present says
* nothing about whether a rule reached the element, and this content is
* portalled out of the plugin's scoped stylesheet.
*/
const openPanel = async (isCompactViewport: boolean) => {
const slot = await render({ isCompactViewport });
fireEvent.click(await slot.findByRole("button", { name: "Thread brief" }));
const panel = (
await slot.findByText("Ship the thread-briefs plugin")
).closest("[style*='max-height']") as HTMLElement | null;
return { slot, panel };
};

const goal = await slot.findByText("Ship the thread-briefs plugin");
const panel = goal.closest("[style*='max-height']") as HTMLElement | null;
it("caps its height and scrolls, so a long brief is reachable on a phone", async () => {
const { slot, panel } = await openPanel(true);
expect(panel).not.toBeNull();
// Without both of these the panel grows past the viewport with no way to
// reach the fields below the fold.
expect(panel?.style.maxHeight).toContain(
"--radix-popover-content-available-height",
);
expect(panel?.className).toContain("overflow-y-auto");
expect(panel?.style.overflowY).toBe("auto");
slot.lifecycle.unmount();
});

it("never exceeds the width Radix measured, so it cannot run off-screen", async () => {
const { slot, panel } = await openPanel(true);
// The bug this replaces: a 100vw-wide panel anchored near the right edge
// hangs off the screen and its text wraps out of sight.
expect(panel?.style.maxWidth).toContain(
"--radix-popover-content-available-width",
);
slot.lifecycle.unmount();
});

it("wraps long unbroken strings rather than widening", async () => {
const { slot, panel } = await openPanel(true);
expect(panel?.style.overflowWrap).toBe("anywhere");
slot.lifecycle.unmount();
});

it("goes near-full-width on a compact viewport and a fixed column otherwise", async () => {
const compact = await render({ isCompactViewport: true });
fireEvent.click(await compact.findByRole("button", { name: "Thread brief" }));
const compactPanel = (
await compact.findByText("Ship the thread-briefs plugin")
).closest("[style*='max-height']") as HTMLElement | null;
expect(compactPanel?.className).toContain("w-[calc(100vw-1rem)]");
compact.lifecycle.unmount();
const compact = await openPanel(true);
expect(compact.panel?.style.width).toBe("calc(100vw - 1rem)");
compact.slot.lifecycle.unmount();

const wide = await render({ isCompactViewport: false });
fireEvent.click(await wide.findByRole("button", { name: "Thread brief" }));
const widePanel = (
await wide.findByText("Ship the thread-briefs plugin")
).closest("[style*='max-height']") as HTMLElement | null;
expect(widePanel?.className).toContain("w-80");
wide.lifecycle.unmount();
const wide = await openPanel(false);
expect(wide.panel?.style.width).toBe("20rem");
wide.slot.lifecycle.unmount();
});

it("sets a manual stage", async () => {
Expand Down
28 changes: 18 additions & 10 deletions plugins/thread-briefs/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -308,19 +308,27 @@ function BriefHeaderAction({
align="end"
sideOffset={6}
collisionPadding={8}
// A five-field brief is easily taller than a phone viewport, so the
// panel has to cap its height and scroll inside. Radix measures the
// room it actually has and publishes it as this variable; the vh
// fallback covers the case where collision detection is skipped.
// Inline rather than a Tailwind arbitrary value so it cannot depend
// on what the plugin's Tailwind pass chose to emit.
// Every layout-critical property is an inline style. The plugin's
// Tailwind output is scoped to its own subtree, and this content is
// portalled, so leaning on those classes for sizing is a bet this
// panel does not need to take. Cosmetics stay in className, where a
// miss is only cosmetic.
style={{
// A five-field brief is easily taller than a phone viewport.
maxHeight: "var(--radix-popover-content-available-height, 70vh)",
// ...and `100vw` is not the room this panel has: it is anchored to
// a trigger near the right edge, so a viewport-wide panel hangs off
// the screen and its text wraps out of sight. Radix measures the
// width actually available from where it was placed; cap by that.
maxWidth: "var(--radix-popover-content-available-width, calc(100vw - 1rem))",
width: isCompactViewport ? "calc(100vw - 1rem)" : "20rem",
overflowY: "auto",
overscrollBehavior: "contain",
// Long unbroken strings (URLs, branch names) must not force the
// panel wider than its cap.
overflowWrap: "anywhere",
}}
className={`z-50 overflow-y-auto overscroll-contain rounded-md border border-border bg-card p-3 shadow-md ${
// Near-full width on a phone; a fixed column on a wide screen.
isCompactViewport ? "w-[calc(100vw-1rem)]" : "w-80"
}`}
className="z-50 rounded-md border border-border bg-card p-3 shadow-md"
>
<BriefBody state={state} onPick={onPick} onRefresh={onRefresh} />
</Popover.Content>
Expand Down
Loading