diff --git a/clients/web/src/components/screens/SkillsScreen/SkillsScreen.stories.tsx b/clients/web/src/components/screens/SkillsScreen/SkillsScreen.stories.tsx index fc86c031..2c480950 100644 --- a/clients/web/src/components/screens/SkillsScreen/SkillsScreen.stories.tsx +++ b/clients/web/src/components/screens/SkillsScreen/SkillsScreen.stories.tsx @@ -263,6 +263,10 @@ export const LongManifest: Story = { // name, a URI with many breakable segments, and a description at the upper end // of what SEP-2640 permits. const HOSTILE_NAME = "an-extremely-long-skill-name-".repeat(8); +// A manifest entry whose FILENAME is hostile, not just its path — selecting it +// puts that name in the Skill Resource control, which is pinned and does not +// scroll. +const HOSTILE_FILE_URI = `skill://${"very-long-path-segment/".repeat(30)}${"a-very-long-file-name-".repeat(10)}.md`; const hostileHeaderSkill: SkillEntry = { uri: `skill://${"very-long-path-segment/".repeat(30)}SKILL.md`, frontmatter: { @@ -275,6 +279,7 @@ const hostileHeaderSkill: SkillEntry = { digest: SELF_DIGEST, size: 8, }, + { uri: HOSTILE_FILE_URI, digest: SELF_DIGEST, size: 8 }, ], }; @@ -323,6 +328,32 @@ export const HostileHeader: Story = { await expect(detailCard.scrollHeight).toBeLessThanOrEqual( detailCard.clientHeight + 1, ); + + // Selecting the resource with the hostile FILENAME puts it in the Skill + // Resource control, which is pinned and does not scroll — so it has to be + // clamped too, or the header grows instead. + // Measure the control while a SHORT name is displayed, then select the + // hostile one: a clamped caption leaves the control the same height, an + // unclamped one grows it. Comparing against its own baseline is what makes + // this detect the defect — an absolute threshold does not, because even an + // unclamped name only wraps to a few lines. + const control = await canvas.findByRole("button", { + name: /Skill Resource/, + }); + const controlBefore = control.getBoundingClientRect().height; + const accordionBefore = accordion.getBoundingClientRect().height; + await userEvent.click( + canvas.getByRole("button", { name: HOSTILE_FILE_URI }), + ); + await expect( + Math.abs(control.getBoundingClientRect().height - controlBefore), + ).toBeLessThanOrEqual(1); + await expect( + Math.abs(accordion.getBoundingClientRect().height - accordionBefore), + ).toBeLessThanOrEqual(1); + await expect(detailCard.scrollHeight).toBeLessThanOrEqual( + detailCard.clientHeight + 1, + ); }, }; diff --git a/clients/web/src/components/screens/SkillsScreen/SkillsScreen.test.tsx b/clients/web/src/components/screens/SkillsScreen/SkillsScreen.test.tsx index 39e41237..7c5843da 100644 --- a/clients/web/src/components/screens/SkillsScreen/SkillsScreen.test.tsx +++ b/clients/web/src/components/screens/SkillsScreen/SkillsScreen.test.tsx @@ -392,7 +392,10 @@ describe("SkillsScreen", () => { // accordion. What is worth pinning here is the contract that makes // clamping safe: the full value stays reachable on a `title`, so nothing // is actually hidden from the user. - expect(screen.getByTitle(hostile.uri)).toBeInTheDocument(); + // Two captions legitimately carry it: the header's URI and the Skill + // Resource control's file name, which for the skill's own SKILL.md is the + // same URI. + expect(screen.getAllByTitle(hostile.uri).length).toBeGreaterThanOrEqual(1); expect( screen.getByTitle(hostile.frontmatter.description as string), ).toBeInTheDocument(); diff --git a/clients/web/src/components/screens/SkillsScreen/SkillsScreen.tsx b/clients/web/src/components/screens/SkillsScreen/SkillsScreen.tsx index 420fc1c5..cb72458d 100644 --- a/clients/web/src/components/screens/SkillsScreen/SkillsScreen.tsx +++ b/clients/web/src/components/screens/SkillsScreen/SkillsScreen.tsx @@ -293,10 +293,41 @@ const SkillUriCaption = Text.withProps({ // documents. Three lines is enough to read a real description; the full text // stays available through the native `title` tooltip the call site passes. const SkillDescription = Text.withProps({ - size: "sm", + variant: "skillDescription", lineClamp: 3, }); +// The Skill Resource control's row. `nowrap` is load-bearing: flexbox wraps +// BEFORE it shrinks, so in a wrapping row a long file name pushes itself onto a +// second line and grows the pinned control no matter how the caption is +// clamped. With nowrap the caption's `miw: 0` can take effect and it ellipsizes +// instead. +const ResourceHeaderRow = Group.withProps({ + justify: "space-between", + wrap: "nowrap", + gap: "sm", + w: "100%", +}); + +// The displayed file's name, in the Skill Resource section's CONTROL — which is +// pinned and therefore does not scroll, so a resource URI ending in a very long +// breakable segment would grow the header itself. One line, with the full name +// on the `title` the call site passes. +const ResourceNameCaption = Text.withProps({ + variant: "monoCaption", + // `truncate` alone is not enough: the caption sits in a wrapping header row, + // so a long name whose MIN-CONTENT width exceeds the space simply pushes + // itself onto a second line and grows the control anyway. `miw: 0` lets flex + // shrink it below that width, which is what actually engages the ellipsis. + truncate: "end", + // `miw: 0` alone was not enough — Mantine's Accordion label wrapper does not + // let the row shrink, so the caption kept its content width and grew the + // control anyway. A hard cap gives the ellipsis a definite width to work + // against, which is what actually bounds the pinned header. + miw: 0, + maw: "50%", +}); + const IssueStack = Stack.withProps({ gap: "xs", }); @@ -1570,12 +1601,14 @@ export function SkillsScreen({ } > - + Skill Resource {previewUri !== undefined && ( - {resourceFileName(previewUri)} + + {resourceFileName(previewUri)} + )} - +