[docs] Fix HTML validation issues from link-check#4734
Conversation
commit: |
Bundle size
PerformanceTotal duration: 1,295.42 ms +27.86 ms(+2.2%) | Renders: 78 (+0) | Paint: 2,037.22 ms +57.26 ms(+2.9%) No significant changes — details Check out the code infra dashboard for more information about this PR. |
- ReleaseTimeline: h3 -> h2 so release cards don't skip a level under the page h1 - Autocomplete grid demo: add aria-label to the message composer input - link-check: permit React's javascript:throw form action sentinel via a form action enum override, and disable heading-level on the accordion page (its Accordion.Header h3 is canonical and can't be scoped away cleanly)
509c759 to
fee0ac4
Compare
canary.75 bakes no-missing-references:off and the form action sentinel override into mui:recommended, so the link-checker only keeps the base-ui-specific accordion heading-level disable.
…ion-fixes # Conflicts: # package.json # pnpm-lock.yaml
- ReleaseTimeline: render the decorative spine as a ::before pseudo-element
instead of a <div> child of the <ul> (only <li> is permitted under <ul>)
- ReferenceAccordion: use React.useId() for the caption id so a hook's
parameter and return-value tables no longer share ${partName}-caption
The reference table trigger is a <button>, so its type cell must contain only phrasing content. Props without a compact shortType fell back to the full type rendered via CodeBlock.PreInline, which wraps the code in a ScrollArea <div> (CodeBlockPreContainer) - invalid inside a button/span and flagged by html-validate's element-permitted-content rule. Add CodeBlock.TypeInline to render the type inline (the highlighted type is already a <code>, and the enclosing Accordion.Scrollable handles overflow), and keep the expanded panel type as a block via DetailedTypePre.
The Avatar page referenced ImageLoadingStatus (via onLoadingStatusChange) as a TypeRef linking to #imageloadingstatus, but never rendered its additional-types block, so the anchor did not exist and the link was broken. Render <TypesAvatarAdditional> like the other pages that expose standalone types.
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| ## Additional Types | ||
|
|
||
| <TypesAvatarAdditional showAdditionalTypes={['imageloadingstatus']} /> | ||
|
|
There was a problem hiding this comment.
I'm a bit lost about this. Why do we need it in Avatar but not in e.g. Drawer (see screenshot below). The TransitionStatus type doesn't link anywhere in Drawer.Popup but ImageLoadingStatus does in Avatar.Root.
We intentionally don't display these types in component pages, so if we could find another solution, that'd be great.
There was a problem hiding this comment.
I was wrong, Toast also has the Additional types section. It's just that it hides it via [//]: # '@exclude-table-of-contents'. We should do the same here if we end up displaying this section in Avatar.
There was a problem hiding this comment.
Why do we need it in Avatar but not in e.g. Drawer
Looks like ImageLoadingStatus is publicly exported, while TransitionStatus is coming from ./internals/
There was a problem hiding this comment.
Removing it breaks the popover. Not 100% sure why.
There was a problem hiding this comment.
Ok, so apparently these popups are populated from a page-wide registry, which in turn gets populated by the reference tables. If we remove AdditionalTypes there is nothing being registered for that type and the popup falls back to a (broken) link. Conceptually it also seems like the section is meant to serve as the target for the anchor link that the popup falls back to for SSR (a bit useless if hidden, but ok)
Enabling html-validate surfaced element-permitted-content: the reference table trigger is a <button>, but complex short types (and class method return values) rendered through a block CodeBlock (a ScrollArea <div>), which is invalid phrasing content. The short type and the full type shared one pre component (TypePre), so the earlier attempt to make it inline also forced the expanded panel's detailed type inline, mangling multi-line types. Patch docs-infra to add a separate ShortTypePre for the shortType field map (and render class method return values through the inline shortType map), then pass CodeBlock.TypeInline as ShortTypePre while keeping TypePre as the block PreInline. Trigger types are now inline; the panel keeps block rendering. The docs-infra patch should be upstreamed to @mui/internal-docs-infra.
Match the Toast page: keep the Additional Types section (so the ImageLoadingStatus anchor exists) but hide it from the quick nav.
There was a problem hiding this comment.
temporary patch until adopted in mui/mui-public#1639
The ShortTypePre slot was upstreamed to @mui/internal-docs-infra (mui-public mui#1639) and published in 0.12.1-canary.10. Bump docs-infra to 0.12.1-canary.12 and remove the local patch.
aarongarciah
left a comment
There was a problem hiding this comment.
Sharing only the interesting findings from Fable 5:
Simplifications
🟡 Methods/Properties accordions keep the deterministic caption-id pattern the PR removes from ReferenceAccordion
Location: docs/src/components/ReferenceTable/MethodsReferenceAccordion.tsx:21
const captionId = `${partName}-methods-caption`;The PR switches ReferenceAccordion to React.useId() because ${partName}-caption duplicated when the same part name rendered twice on a page. MethodsReferenceAccordion and PropertiesReferenceAccordion (${partName}-properties-caption, [MethodsReferenceAccordion.tsx:21](docs/src/components/ReferenceTable/MethodsReferenceAccordion.tsx:21), [PropertiesReferenceAccordion.tsx:21](docs/src/components/ReferenceTable/PropertiesReferenceAccordion.tsx:21)) have the identical latent bug — and ReferenceTable renders MethodsReferenceAccordion twice with the same name (static methods + instance methods, [ReferenceTable.tsx](docs/src/components/ReferenceTable/ReferenceTable.tsx)). No currently documented class has both, so no-dup-id passes today.
Failure scenario: The first documented class with both static and instance methods emits two X-methods-caption ids and fails the newly enabled no-dup-id check in CI, in a PR that has nothing to do with the docs infra.
Fix: Switch both components to React.useId() in this PR, matching ReferenceAccordion.
Docs
ℹ️ TypeInline drops all props except children, unlike its TypePre counterpart
Location: docs/src/components/CodeBlock/CodeBlock.tsx:149
export function TypeInline({ children }: { children?: React.ReactNode }) {
return <span className="Code TableCode">{children}</span>;
}TypePre (PreInline) forwards full React.ComponentProps<'pre'>, but TypeInline accepts only children. If the docs-infra factory ever passes className or data attributes to ShortTypePre, they're silently discarded; the hardcoded "Code TableCode" string also duplicates the class names owned by the Code/TableCode components. Couldn't verify the factory contract locally (new canary not installed), so this is a heads-up only.
Verdict
Approve after nits — no correctness issues; the nits are a latent duplicate-id trap in the sibling accordions, a missing test for the new slug-dedupe logic, and a heading-case inconsistency.
Switch MethodsReferenceAccordion and PropertiesReferenceAccordion to
React.useId() for the caption id, matching ReferenceAccordion. The
deterministic ${partName}-caption pattern duplicates when a class
renders both static and instance methods, tripping no-dup-id in CI.
Flip the ReleaseTimeline nth-child parity so the first card is aligned
left again: removing the leading .TimelineSpine div shifted every
item's parity.
…ng case Add a rehypeSlug test covering the new duplicate-slug suffixing. Forward props on CodeBlock.TypeInline and merge className, matching PreInline. Use sentence case for the 'Additional types' headings.
…ion-fixes # Conflicts: # docs/package.json # package.json # pnpm-lock.yaml
aarongarciah
left a comment
There was a problem hiding this comment.
Fable 5 only found one remaining issue (apart from the CI failing). I tested manually and couldn't any issues.
2. 🟡 rehypeSlug dedup ignores pre-existing heading ids, so collisions with them remain possible
Location: docs/src/components/QuickNav/rehypeSlug.mjs:46
visit(tree, 'element', (node) => {
if (headingRank(node) && !node.properties.id) {
...
occurrences.set(id, 0);
node.properties.id = id;
}Headings that already carry an id (set by an author or an earlier plugin) are skipped entirely and never seeded into the occurrences map. A later heading whose text slugs to that same value still gets the bare slug, reintroducing exactly the duplicate-id class of bug this PR fixes — and it would surface as a confusing no-dup-id CI failure now that the rule is enforced. The new test suite covers the "already has an id" passthrough but not this collision case.
Failure scenario: A page contains <h2 id="props"> (explicit) followed by ## Props; both end up with id="props", the TOC link targets the wrong heading, and the link-check job fails on no-dup-id.
Fix: Seed occurrences with existing heading ids as they are visited (e.g. occurrences.set(node.properties.id, 0) in an else branch), and add a test for the case.


Enables html-validate's
mui:recommendedruleset in the docs link-check and fixes all issuesComponent/demo fixes:
Table:TableCellInnerspan → div (block content no longer under<span>).ReferenceTable: render fallback type signatures inline so the type cell no longer nests a blockCodeBlockdiv in the trigger<button>; h4 → h3 for contiguous heading levels; unique caption id per table instance.rehypeSlug: dedupe heading ids (no-dup-id). FixesmergePropslink in TOC on https://base-ui.com/react/utils/merge-propsCodeBlock: defaultcodeId/titleIdtoundefined(noid=""outside aCodeBlock.Root).ReleaseTimeline: card h3 → h2; spine as a::beforepseudo-element (no<div>under<ul>).DemoPlayground: addrole="figure"to the labelled demo container.<TypesAvatarAdditional>so theImageLoadingStatusTypeRef resolves (its#imageloadingstatusanchor was never emitted → broken link).aria-labelon the composer and search inputs.