Skip to content

fix(css-syntax): always link <type> references in formal syntax#685

Draft
caugner wants to merge 1 commit into
mainfrom
165-fix-css-syntax-link-constituent-types
Draft

fix(css-syntax): always link <type> references in formal syntax#685
caugner wants to merge 1 commit into
mainfrom
165-fix-css-syntax-link-constituent-types

Conversation

@caugner
Copy link
Copy Markdown
Contributor

@caugner caugner commented May 12, 2026

Description

Always wrap <type> references in formal-syntax boxes with a link to the corresponding Reference/Values/{slug} page, even when the type's definition is also expanded inline as a constituent.

Motivation

Previously, types whose definitions were expanded in the same syntax box (e.g. <calc-keyword> inside calc(), <length-percentage> inside padding) were rendered without a link, on the assumption that the inline expansion made navigation redundant. This produced inconsistent UX with sibling <type> refs that do get linked.

Additional details

  • Drops the self.constituents.contains(node) short-circuit in the Node::Type branch of SyntaxRenderer::render_node; only the existing skip() cases (color, gradient) remain unlinked.
  • Updated snapshot expectations in test_render_node and test_render_function_scoped.
  • Follow-up commit removes the now-unused SyntaxRenderer::constituents field and the Constituent wrapper struct.
Before After
image image

Related issues and pull requests

Fixes #165.

@caugner
Copy link
Copy Markdown
Contributor Author

caugner commented May 12, 2026

@Josh-Cena @Rumyra Can you please take a look at the screenshots above and let me know if that makes sense? Basically, linking to types unconditionally adds 7 links in the above example. (I think it's fine.)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 12, 2026

5f5287b was deployed to: https://rari-pr685.review.mdn.allizom.net/

In formal-syntax boxes, `<type>` references whose definitions are
expanded inline below previously rendered without a link, on the
assumption that the inline expansion made navigation redundant. This
left occurrences like `<calc-keyword>` inside `calc()` looking
inconsistent with sibling `<type>` refs that *do* link out.

Instead of linking every body occurrence (which produces 4+ identical
links on pages like `calc()`), link the expansion *heading*: the
`<calc-keyword> = …` line itself becomes an anchored link to the
type's Reference page. One link per expanded type, placed at its
definition site.

- Adds `SyntaxLine::heading_url`, populated by `get_constituent_syntaxes`
  for Type/Function/Property/AtRule constituents (`None` for the
  top-level syntax, whose heading is the current page).
- `SyntaxRenderer::render` wraps the heading name in `<a>` when
  `heading_url` is set.
- Updated snapshot expectations in `test_render_node` and
  `test_render_function_scoped`.

Fixes #165
@caugner caugner force-pushed the 165-fix-css-syntax-link-constituent-types branch from 2f32afe to aeb111a Compare May 12, 2026 15:53
@caugner caugner changed the title fix(css-syntax): always link <type> references in formal syntax fix(css-syntax): link constituent expansion headings May 12, 2026
@caugner
Copy link
Copy Markdown
Contributor Author

caugner commented May 12, 2026

Basically, linking to types unconditionally adds 7 links in the above example.

Instead, we could only link the heading of the expanded constituent types (note: this would need some style adjustments in fred):

image

@caugner caugner changed the title fix(css-syntax): link constituent expansion headings fix(css-syntax): always link <type> references in formal syntax May 12, 2026
@Josh-Cena
Copy link
Copy Markdown
Member

Does the red squiggly mean "broken"? Is the intention that this page should be eventually written (which I doubt will; a lot of these types are strictly internal and purely for spec convenience)?

@caugner
Copy link
Copy Markdown
Contributor Author

caugner commented May 12, 2026

In Rari a broken link is just a link where we know that the target does not exist, so the link rendering is replaced by a placeholder element. How that element is styled is up to Fred, and currently these are displayed with squiggly underline.

@Josh-Cena
Copy link
Copy Markdown
Member

Per my understanding, there are far more types in the CSS spec than we'd like to document; most of them are only ever used once or twice as an intermediate form that composes with other stuff. So:

  • If the type already has a documentation page, great, link to it no matter if it's expanded on this page or not;
  • If the type has no documentation page, I think creating a link for it (no matter how it's styled) makes less sense than not linking to it, because it's more likely that a page will never exist.

@caugner
Copy link
Copy Markdown
Contributor Author

caugner commented May 12, 2026

  • If the type already has a documentation page, great, link to it no matter if it's expanded on this page or not;

One important question is where it should be linked:

  1. Only on the expansion.
  2. Only on the 1st reference.
  3. On the expansion and each reference.

Otherwise it is absolutely possible to only link where a page exists, but that is a slightly different issue as #165 where links on expanded attributes are currently missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should other attributes be linked for consistency in the formal syntax?

3 participants