Skip to content

Timeline: tag event stories with taxonomy data-* and split the taxonomy into core and GitHub layers - #8216

Closed
janmaarten-a11y wants to merge 9 commits into
mainfrom
janmaarten-a11y-timeline-lc-taxonomy-data-attrs
Closed

Timeline: tag event stories with taxonomy data-* and split the taxonomy into core and GitHub layers#8216
janmaarten-a11y wants to merge 9 commits into
mainfrom
janmaarten-a11y-timeline-lc-taxonomy-data-attrs

Conversation

@janmaarten-a11y

@janmaarten-a11yjanmaarten-a11y commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Part of github/primer#6664 (epic github/primer#6654). Everything here is internal to the Timeline module and Storybook-only: there is no consumer-facing change, nothing is added to the package's public exports, and there is no changeset.

This PR does two things. It tags the five Timeline surface story sets (License Compliance, Code Scanning, Dependabot, Secret Scanning, Issues) with the taxonomy data-* attributes through one shared helper, and it organizes the taxonomy folder into two clearly separated layers: a generic core that any product could reuse, and a GitHub-specific layer that holds GitHub's own values. The taxonomy folder encodes GitHub product knowledge (which surfaces are security alerts, the per-surface event catalogs, which logins are bots), and the team agreed that Primer should own the generic data-* schema and its serializer while the GitHub-specific values live in a wrapper that can eventually move into github-ui. This split draws that line now, while the module is still internal, so the later move is a small, contained step.

The two layers

  • taxonomy/core is generic and product-agnostic. It holds the data-* attribute types, the serializer that turns an event into its attribute set, and a new factory, createEventDataAttributesFor, that binds a catalog (and an optional rule for turning a login into a person-or-bot value) into a ready-to-use helper. Nothing in core knows about GitHub surfaces, categories, or bot logins.
  • taxonomy/github is the GitHub-specific layer. It holds the surfaces, the category model, the five per-surface event catalogs, the rule that classifies a login as a person or a bot, and eventDataAttributesFor, which is now a single line that instantiates the core factory with the GitHub catalog and the GitHub actor rule.
  • taxonomy/index re-exports both layers, so every symbol the stories rely on, and eventDataAttributesFor in particular, stays available from the same place as before. The five surface story files import it exactly as they did and did not change.

What each row carries

Every event row spreads the data-* set produced by eventDataAttributesFor(scope, type, login?): data-event-scope, data-event-type (the unscoped leaf), data-event-category and data-event-visibility (both read from that surface's catalog, so the stories stay in sync with the source of truth instead of hand-typing values), and data-actor-type (user or bot, resolved from the login and omitted when the row has no actor). The helper is type-safe: the compiler enforces that the event type is a real leaf of the given surface, and it never emits data-actor-type for a structurally actor-less event even if a login is passed.

Behaviour is unchanged

  • The emitted data-* attributes are byte-identical before and after the split. Every event story across the five files was rendered and every row that carries data-event-scope had its data-* values compared against a baseline captured before the change; the difference is none across all 162 rows.
  • The five Timeline surface story files are untouched (zero changed lines).
  • The taxonomy stays internal, so there is no public export change and no changeset.

Testing

The repo's own gates pass: prettier, eslint with zero warnings allowed, and type-check. The taxonomy tests pass, including new core tests for the serializer and the factory (person-versus-bot resolution, the omitted-actor case, the actor-less suppression, auditOnly pass-through, and category derivation from the catalog) alongside the moved surface tests. A render-and-diff equivalence check across every tagged row confirmed the reorganization changed no attributes.

@changeset-bot

changeset-botBot commented Jul 22, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 77bc258

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@janmaarten-a11yjanmaarten-a11y added skip changeset This change does not need a changelog integration-tests: skipped manually Changes in this PR do not require an integration test component: Timeline labels Jul 22, 2026
@github-actions
github-actionsBottemporarily deployed to storybook-preview-8216 July 22, 2026 18:48 Inactive
@janmaarten-a11yjanmaarten-a11y changed the title Timeline: tag License Compliance event stories with taxonomy data-* attributesTimeline: tag event stories with taxonomy data-* attributes (all five surfaces)Jul 22, 2026
@github-actions
github-actionsBottemporarily deployed to storybook-preview-8216 July 22, 2026 22:20 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-8216 July 24, 2026 19:50 Inactive
@janmaarten-a11y
janmaarten-a11yforce-pushed the janmaarten-a11y-timeline-lc-taxonomy-data-attrs branch from 0196145 to e5e13b9CompareJuly 24, 2026 20:01
@janmaarten-a11yjanmaarten-a11y changed the title Timeline: tag event stories with taxonomy data-* attributes (all five surfaces)Timeline: tag event stories with taxonomy data-* attributes (helper, all five surfaces, gap fixes)Jul 24, 2026
@github-actions
github-actionsBottemporarily deployed to storybook-preview-8216 July 24, 2026 20:05 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-8216 July 24, 2026 20:17 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-8216 July 24, 2026 20:29 Inactive

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR standardizes Timeline Storybook event stories by projecting taxonomy data-* attributes from the shared Timeline taxonomy catalogs, consolidating the per-surface attribute logic into a single internal helper, and closing a few taxonomy/story gaps discovered during the tagging pass.

Changes:

  • Added a reusable internal helper (eventDataAttributesFor) to generate the Timeline event taxonomy data-* attributes from the per-surface catalogs.
  • Updated all five Timeline event story surfaces to tag cataloged rows via the helper (leaving intentionally-uncataloged rows untagged and documented in-line).
  • Expanded taxonomy coverage/behavior: added Secret Scanning reported leaf and classified the GitHub system identity (github/GitHub) as a bot.
Show a summary per file
FileDescription
packages/react/src/Timeline/Timeline.secret-scanning.features.stories.tsxApplies taxonomy data-* attributes to cataloged Secret Scanning story rows via the shared helper; documents intentional untagged variants.
packages/react/src/Timeline/Timeline.license-compliance.features.stories.tsxApplies taxonomy data-* attributes to all License Compliance rows via the helper (fully cataloged surface).
packages/react/src/Timeline/Timeline.issues.features.stories.tsxTags Issues rows via the helper; removes the invalid “Converted from draft” variant; documents intentional untagged variants.
packages/react/src/Timeline/Timeline.dependabot.features.stories.tsxTags Dependabot cataloged rows via the helper; documents shared/parked untagged groups (assignment/copilot work).
packages/react/src/Timeline/Timeline.code-scanning.features.stories.tsxTags Code Scanning rows via the helper and clarifies taxonomy mapping for visually-grouped variants.
packages/react/src/Timeline/taxonomy/index.tsRe-exports the new helper from the Timeline taxonomy module.
packages/react/src/Timeline/taxonomy/eventTaxonomy.tsAdds Secret Scanning reported leaf and clarifies Secret Scanning taxonomy comments.
packages/react/src/Timeline/taxonomy/eventTaxonomy.test.tsExtends bot-login classification coverage to include github.
packages/react/src/Timeline/taxonomy/eventDataAttributes.tsIntroduces eventDataAttributesFor helper that projects data-* attributes from SURFACE_TAXONOMIES.
packages/react/src/Timeline/taxonomy/eventDataAttributes.test.tsAdds unit tests for helper behavior (actor resolution, omissions, auditOnly preservation, category derivation).
packages/react/src/Timeline/taxonomy/actorType.tsClassifies the github system identity as a bot for data-actor-type purposes.

Review details

Suppressed comments (3)

packages/react/src/Timeline/Timeline.issues.features.stories.tsx:582

  • This JSX comment is a single very long line, which hurts readability and can trip max-len linting. Please wrap it to match the surrounding comment style used throughout this stories file.
 {/* Untagged: verified against the redesign prototype (github/prototyping janmaarten-a11y/timeline-redesign-v1) as an intentional exclusion, this variant is not modeled as a distinct catalog leaf (the catalog has only 'marked_as_duplicate'); no leaf -> no data-* tag. */}

packages/react/src/Timeline/Timeline.issues.features.stories.tsx:604

  • This JSX comment is a single very long line, which hurts readability and can trip max-len linting. Please wrap it to match the surrounding comment style used throughout this stories file.
 {/* Untagged: verified against the redesign prototype (github/prototyping janmaarten-a11y/timeline-redesign-v1) as an intentional exclusion, this variant is not modeled as a distinct catalog leaf (the catalog has only 'marked_as_duplicate'); no leaf -> no data-* tag. */}

packages/react/src/Timeline/Timeline.issues.features.stories.tsx:1845

  • This JSX comment is a single very long line, which hurts readability and can trip max-len linting. Please wrap it to match the surrounding comment style used throughout this stories file.
 {/* Untagged: a rolled-up event (labeled + unlabeled in one row) verified against the redesign prototype (github/prototyping janmaarten-a11y/timeline-redesign-v1) as an intentional exclusion, not modeled as a distinct catalog leaf; no leaf -> no data-* tag. */}
  • Files reviewed: 11/11 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment threadpackages/react/src/Timeline/Timeline.issues.features.stories.tsx Outdated
janmaarten-a11yand others added 7 commits August 10, 2026 14:54
…ttributes
Spread the event data-* contract from the taxonomy module onto every
Timeline.Item in the License Compliance feature stories. This is the
proof-of-pattern pilot for Phase 3 taxonomy tagging (github/primer#6664).
Storybook-only, no consumer-facing change.
Copilot-Session: ac5d1407-e965-4994-b6d5-fd1b8ec0cdef
…event stories with taxonomy data-* attributes
Extend the taxonomy data-* tagging from License Compliance to the
remaining four surfaces, so all five now carry the contract. Same
approach: each Timeline.Item spreads toEventDataAttributes(), deriving
category and visibility from that surface's catalog and resolving
data-actor-type from the rendered login. Parked and shared events, and
variants with no catalog leaf, are left untagged with inline comments.
Storybook-only; no visual change.
converted_from_draft is a real issue-timeline event (Projects v2 draft-issue conversion; github/github issue_timeline_items union). It has no taxonomy catalog leaf yet, so the row is restored untagged, matching the other uncataloged variants, rather than removed.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Separate the internal Timeline taxonomy module into a product-agnostic core and a GitHub-specific layer. The core (taxonomy/core) holds the generic data-* schema, the serializer, and a new createEventDataAttributesFor factory. The GitHub layer (taxonomy/github) holds the concrete surfaces, category model, per-surface event catalogs, actor-classification rule, and the bound eventDataAttributesFor helper, which is now a one-line instantiation of the core factory.
The emitted data-* attributes stay byte-identical, the taxonomy barrel keeps the same external symbols, and the module remains internal (no public export, no changeset).
@janmaarten-a11y
janmaarten-a11yforce-pushed the janmaarten-a11y-timeline-lc-taxonomy-data-attrs branch from b250250 to d080c50CompareAugust 10, 2026 21:56
@janmaarten-a11yjanmaarten-a11y changed the title Timeline: tag event stories with taxonomy data-* attributes (helper, all five surfaces, gap fixes)Timeline: add taxonomy data-* attributes to the event stories and split the taxonomy into a generic core and a GitHub layerAug 10, 2026
@github-actions
github-actionsBottemporarily deployed to storybook-preview-8216 August 10, 2026 22:07 Inactive
Address code-review nits on the taxonomy core/github split, all comment and type only with no behavior change. Drop the now-redundant `as EventTaxonomyEntry` cast in the core factory (the generic catalog constraint already resolves the entry type). Declare the factory's projector signature once by inferring the function's return type from its fully-typed arrow instead of repeating the annotation. Reword the core serializer docs so they describe serializing an already-projected input rather than modeling the taxonomy axes. Call the taxonomy barrel a module entry point rather than a public one, since it stays internal. Give the GitHub catalogs file a header that names the per-surface event catalogs it holds so the filename and contents read consistently.
@janmaarten-a11yjanmaarten-a11y changed the title Timeline: add taxonomy data-* attributes to the event stories and split the taxonomy into a generic core and a GitHub layerTimeline: tag event stories with taxonomy data-* and split the taxonomy into core and GitHub layersAug 10, 2026
Flow the stranded word in the core serializer module header so the sentence about the GitHub-specific values reads on continuous lines. Comment only, no code or behavior change.

@liuliu-devliuliu-dev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this PR can deliver the Storybook playground from #6664, while keeping the GitHub event taxonomy in github-ui.

I’d suggest refocusing the PR on an interactive playground that uses a small set of representative example events defined within the story, filters the data before rendering, and passes representative data-* attributes to Timeline.Item. The filter control could be composed from ActionMenu and ActionList. This would demonstrate how consumers can build Timeline filtering without making Primer the source of truth for GitHub event categories.

@janmaarten-a11y

Copy link
Copy Markdown
ContributorAuthor

Closing this without merging. The team has decided the Timeline event taxonomy should live in github-ui rather than Primer, so Primer stays taxonomy-agnostic and ships only the Timeline component and, later, an optional presentational Timeline.Filter control. That removes the purpose of this PR, which tags Primer stories with GitHub-specific data-* values and reorganizes a GitHub taxonomy inside Primer, both of which now belong in github-ui.

Nothing is lost. This branch stays in git history and carries the most complete version of the model, with the generic core and the GitHub-specific catalogs split into separate layers, and github-ui will lift the taxonomy from here. The per-surface event research that informed it is preserved and will feed that work.

Next steps are tracked in #6664: follow-ups adds the playground, representative story data, the filter controls, and the data-* attributes from github-ui.

The taxonomy module merged into Primer in #8180 is being reverted separately.

@siddharthkp

Copy link
Copy Markdown
Member

@janmaarten-a11y Thanks for leaving a good comment here instead of just closing it ❤️

@primer
primerBot deleted the janmaarten-a11y-timeline-lc-taxonomy-data-attrs branch August 20, 2026 00:04
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: Timelineintegration-tests: skipped manuallyChanges in this PR do not require an integration testskip changesetThis change does not need a changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@janmaarten-a11y@siddharthkp@liuliu-dev@francinelucca