fix(compat-table): group support history by prefix/altname#1587
Merged
Conversation
Support arrays in BCD interleave parallel implementations (e.g. unprefixed, `-webkit-`, `-moz-`) in canonical-first order rather than chronologically. Reversing the whole array for the timeline produced out-of-order versions (e.g. Firefox 10 → 49 → 16 for `perspective`). Group items by `(prefix, alternative_name)` and render each branch as its own timeline so versions stay chronological within a branch. Each branch gets its own timeline line spanning only its first to last icon, and non-canonical branches carry a heading identifying the modifier.
Move the `prefix` and `altname` icons from each timeline row up to the branch heading, where they sit next to the "With `…` prefix" label. This avoids repeating the modifier on every version line within the branch and gives the heading a clearer visual anchor. Threaded via a new `omitModifiers` option on `_renderCellText` / `_renderCellIcons`, applied only when the branch heading is shown.
…te name: …` Reads more like a label than a sentence and keeps the modifier value visually anchored at the end where the eye lands after the icon.
…ranch
When a branch heading conveys the `prefix` / `alternative_name`, the
timeline row drops the modifier note. The full-support check still
treated those properties as a limitation though, so a bare
`{ prefix, version_added }` item (e.g. Chrome 12 with `-webkit-`)
fell through to the "Support unknown" fallback. Strip the modifier
fields before the check when `omitModifiers` is set.
Previously the heading only appeared when multiple branches were present, so a single-branch case like Opera Android supporting `crisp-edges` only via the alternate name `-webkit-optimize-contrast` rendered the modifier as a timeline note instead. Show the heading whenever a branch is non-canonical (has `prefix` or `alternative_name`) and suppress the redundant timeline note accordingly.
…lumns Use subgrid on the timeline row and place the notes list in `grid-column: 2 / -1` so each branch box spans exactly the browser columns, leaving the feature-name column free. Drop the previous `margin-left: 20%` / `max-width: 80vw` approximation. Adjacent branches share a single 1px separator; the surrounding table cell and a left border on the notes list provide the outer edges.
The desktop layout places `.bc-notes-list` in `grid-column: 2 / -1` and adds a left border to separate it from the feature-name column. On mobile the table is a single-column grid, so column 2 doesn't exist — override the placement back to span the full width and drop the now- irrelevant left border. Also zero the branch's horizontal padding to use the available width, and mirror the existing `margin-left` on `.timeline` with a matching `margin-right`.
Contributor
|
f44f9a6 was deployed to: https://fred-pr1587.review.mdn.allizom.net/ |
…pportBranches` Drop the comparator-based sort in favor of an explicit lookup + prepend. Avoids depending on sort stability and reads more directly.
…ches
Each Map entry is created with its first item and only mutated by `push`,
so branches are non-empty by construction. Encode that in the return type
as `[T, ...T[]][]` and drop the impossible `?? {}` fallback at the call
site.
…ortedWithoutLimitation` Previously the call site spread `item` with `prefix: undefined, alternative_name: undefined` to bypass the modifier check. Push the policy into the predicate so the caller asks for the semantic it wants instead of mutating a copy of the item.
The branch box drops its horizontal padding on mobile and reaches the cell edges, so the timeline now needs matching margin on both sides.
…AliasModifiers` `Modifier` alone was vague; `aliasModifier` makes the closure over `prefix` and `alternative_name` (i.e. the two BCD fields that name a feature differently) explicit at the call sites.
…ading` `_renderBranchHeading` is only called when at least one of `prefix` or `alternativeName` is truthy (gated by `hasAliasModifier` in `_renderNotes`), so the three branches cover that invariant exhaustively. Inline the `l10n.raw` calls as a chained ternary and document the invariant in the JSDoc.
Branches are non-empty by construction (`groupSupportBranches` returns `[Simple, ...Simple[]]`), and the `i === 0` arm of the inner `flatMap` always emits a wrapper for the first item — so `wrappers.length > 0` is always true and the outer `.filter(Boolean)` never strips anything.
…ortBranches` `/` reads more naturally than `\0` and is just as safe: BCD prefixes (e.g. `-webkit-`) and alternative names (identifier-shaped) never contain it.
\"Even if it's the only branch\" was ambiguous about which case it addressed. Reword to spell out the actual edge case: a single prefixed/altname branch with no canonical sibling still gets a heading.
Co-locate the heading styles with the other `.bc-branch-*` selectors so all branch-related rules sit in the same nested block. The selector only ever matches inside `.bc-notes-list`, so nesting matches the actual scope.
Since `_renderNotes` groups items by `(prefix, alternative_name)` and sets `omitAliasModifiers = true` whenever a branch carries either modifier, the `compat-support-prefix` and `compat-support-altname` note branches in `_getNotes` can never fire: - non-canonical branches: `omitAliasModifiers` is true, so the `!omitAliasModifiers` guard rejects. - canonical branches: items share `(prefix, alternative_name) = (undefined, undefined)`, so the `item.prefix` / `item.alternative_name` guard rejects. Drop the dead branches and the now-unused `compat-support-prefix` / `compat-support-altname` FTL keys (template + locales). The branch heading conveys the modifier instead.
…pers
After grouping items into per-branch timelines, the `<dl>` parent gained
extra wrapper layers (`bc-branch`, `bc-branch-heading`, `bc-branch-items`)
between itself and the `<dt>`/`<dd>` pairs. That nesting is outside the
`<dl>` content model. The list also wasn't a true term/definition
association — it's a chronological version log. Switching to plain divs
drops the misuse and lets the CSS target classes directly.
The audit also surfaced a dead rule: `dl.bc-notes-list dt.bc-supports`
matched only the first child of `.bc-notes-wrapper`, which was always
the `dt`, so `margin-top: 1rem` + `&:first-child { margin-top: 0 }`
cancelled out. Removed along with the obsolete `dl`-scoped block; the
`:last-child` margin-reset for the last note row is now folded into the
`.bc-supports-dd` block. The empty placeholder gains a `bc-notes-end`
class to carry its own bottom-margin rule.
The per-branch timeline line and the cap pseudo that hides it under the last icon used unrelated magic numbers (4px / 3px). Both now read from `--compat-timeline-cap`, which also unifies the inset to 4px in both directions so a future tweak only needs to touch one place.
`branches.values()` returned entries in BCD insertion order, but BCD doesn't guarantee a stable order for parallel implementations. Sorting by `(alternative_name, prefix)` makes the rendered branch sequence predictable across feature edits to the underlying data.
Pick the message id with a ternary, then call `l10n.raw` once. Fluent silently ignores extra args, so passing both `prefix` and `altname` through regardless of which the chosen id consumes is safe and lets the shared `elements` map live in one literal.
The condition suggests filtering, but its purpose is the opposite: keep the first row of each branch even when it has no notes, so the branch never renders an empty timeline.
caugner
changed the title
Contributor
|
I think this is a really good solution |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Updates the
compat-tablecomponent, rendering each parallel support branch (unprefixed,-webkit-,-moz-, alternate names) as its own cell-like timeline, instead of interleaving them in a single reversed list.Motivation
BCD orders support items canonical-first rather than chronologically. Blindly reversing the array for the timeline produced out-of-order versions: e.g.
css.properties.perspectiveon Firefox displayedFirefox 10 → 49 → 16. Grouping by(prefix, alternative_name)keeps versions chronological within each branch.Additional details
groupSupportBrancheshelper partitions a support statement by modifier; canonical branch first, others sorted by(alternative_name, prefix)for stable rendering.grid-column: 2 / -1), leaving the feature-name column free; stacked branches share a 1px separator.-webkit-" / "Alternate name:…" heading with the modifier icons relocated from the timeline rows, avoiding redundancy.{ prefix, version_added }item fell through toSupport unknown.grid-columnto1 / -1, drops the left border, and zeroes the horizontal padding.Screenshots:
Related issues and pull requests
Fixes mdn/mdn#831.