diff --git a/clients/web/src/components/screens/SkillsScreen/SkillsScreen.stories.tsx b/clients/web/src/components/screens/SkillsScreen/SkillsScreen.stories.tsx index b60e248e..89540860 100644 --- a/clients/web/src/components/screens/SkillsScreen/SkillsScreen.stories.tsx +++ b/clients/web/src/components/screens/SkillsScreen/SkillsScreen.stories.tsx @@ -197,6 +197,80 @@ const LONG_SKILL_MD = [ * actually presented — the viewer's content-sized `flex-basis` crushed its * siblings, so collapsing it laid out correctly and reopening it broke again. */ +// A conforming manifest may declare up to 512 files. This is the sibling case +// to a long document: the metadata section, not the viewer, is what holds the +// overflowing content. +const manyFilesSkill: SkillEntry = { + uri: "skill://big-manifest/SKILL.md", + frontmatter: { + name: "big-manifest", + description: "A conforming skill that declares a great many files", + }, + resources: [ + selfEntry("big-manifest"), + ...Array.from({ length: 120 }, (_, i) => ({ + uri: `skill://big-manifest/file-${String(i).padStart(3, "0")}.md`, + digest: REF_DIGEST, + size: 15, + })), + ], +}; + +/** + * The other half of the layout contract: a huge **manifest**, rather than a + * huge document. + * + * A section that keeps its full intrinsic height pushes the file viewer off the + * bottom of the pane, so reaching the file means scrolling past the manifest — + * which is the "the file is behind the manifest" problem this screen exists to + * end. The metadata sections must therefore shrink to their floor and scroll + * internally, leaving the viewer on screen. + */ +export const LongManifest: Story = { + args: { + skills: [manyFilesSkill], + onReadSkillFile: fn(async () => ({ + text: "---\nname: big-manifest\n---\n\n# Big manifest\n", + mimeType: "text/markdown", + })), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByText("big-manifest")); + const viewerControl = await canvas.findByRole("button", { + name: /Skill Resource/, + }); + + const detailCard = canvasElement.querySelectorAll(".mantine-Card-root")[1]; + if (!(detailCard instanceof HTMLElement)) { + throw new Error("Detail card not found"); + } + + // The viewer's header is ON SCREEN, not pushed below the manifest. + const cardRect = detailCard.getBoundingClientRect(); + const viewerRect = viewerControl.getBoundingClientRect(); + await expect(viewerRect.bottom).toBeLessThanOrEqual(cardRect.bottom + 1); + + // The manifest section gave up space rather than keeping its full height, + // so its own panel is what scrolls. + const resourcesControl = canvas.getByRole("button", { name: /Resources/ }); + const resourcesPanel = resourcesControl + .closest(".mantine-Accordion-item") + ?.querySelector(".mantine-Accordion-panel"); + if (!(resourcesPanel instanceof HTMLElement)) { + throw new Error("Resources panel not found"); + } + await expect(resourcesPanel.scrollHeight).toBeGreaterThan( + resourcesPanel.clientHeight, + ); + + // And the pane still does not scroll as one column. + await expect(detailCard.scrollHeight).toBeLessThanOrEqual( + detailCard.clientHeight + 1, + ); + }, +}; + export const LongSkillDocument: Story = { args: { onReadSkillFile: fn(async () => ({ diff --git a/clients/web/src/components/screens/SkillsScreen/SkillsScreen.tsx b/clients/web/src/components/screens/SkillsScreen/SkillsScreen.tsx index 875af397..b0bfa624 100644 --- a/clients/web/src/components/screens/SkillsScreen/SkillsScreen.tsx +++ b/clients/web/src/components/screens/SkillsScreen/SkillsScreen.tsx @@ -351,20 +351,30 @@ const SkillTitle = Text.withProps({ }); /** - * Per-section flex for the metadata sections: **content height, never shrink**. + * Per-section flex for the metadata sections: content height, but **able to + * shrink to the `mih` floor**. * - * `ResourceControls` weights its shrink by item count (#1462) because its - * panels hold uniform lists that degrade gracefully when squeezed. These do - * not — a findings list, a manifest table with alerts under it, a frontmatter - * block — and squeezing them sliced content mid-line: a `Digest mismatch` - * alert cut in half by the section header below it. The panel was scrollable, - * but a macOS overlay scrollbar is invisible until hover, so it read as broken. + * This setting has been wrong in both directions, so both are recorded. * - * Sized to content instead, with the overflow handled once at the accordion - * root (`skillSections`), the stack scrolls at a *section boundary* rather than - * through the middle of a finding. + * Weighting the shrink by item count, as `ResourceControls` does (#1462), let a + * section be squeezed far below its content and slice it mid-line — a `Digest + * mismatch` alert cut in half by the header beneath it. The panel really was + * scrollable, but a macOS overlay scrollbar is invisible until hover, so it + * read as broken. + * + * Refusing to shrink at all fixed that and reintroduced the original bug at + * scale: a *conforming* manifest may declare 512 rows, and a section holding + * its full intrinsic height pushes the file viewer off the bottom, so reaching + * the file means scrolling past the manifest — precisely the "the file is + * behind the manifest" problem this screen was refactored to end. + * + * `0 1 auto` with the `OPEN_SECTION_MIN_HEIGHT` floor is the setting that + * satisfies both: a section gives up space until it hits the floor, its panel + * scrolls internally from there, and the viewer keeps the remainder. The floor + * is what stops the shrink becoming a crush; nothing above it is sliced, + * because a panel at the floor scrolls rather than clips. */ -const SECTION_FLEX = "0 0 auto"; +const SECTION_FLEX = "0 1 auto"; /** Every section this screen can render, in display order. */ const ALL_SECTIONS = ["conformance", "resources", "frontmatter", "resource"];