Replace Timeline Playground with Custom Event playground#7836
Replace Timeline Playground with Custom Event playground#7836janmaarten-a11y wants to merge 10 commits into
Conversation
Recreates the Figma 'Custom event' component (Primer-Web library, node 46191-13560) as the Timeline Playground story. Composes existing public primitives only (Timeline, Timeline.Item, Timeline.Badge, Timeline.Body, Avatar, Octicon, Link, RelativeTime) — no public API changes.
Storybook controls are grouped into Actor / Badge / Event / Optional content / DOM attributes categories. Highlights:
- Actor: small (20px inline) vs large (40px in left gutter); user / bot / app / copilot types with baked-in canonical names for bot and copilot; user-only avatar URL override
- Badge: 32 octicons + all 9 TimelineBadgeVariant colors
- Event: 5 timestamp presets matching the Figma options (3 relative, 2 absolute) with appropriate render modes (literal / RelativeTime / Intl.DateTimeFormat)
- Optional content: showNote + noteText for second-line cited reasons; viaApp + paired GitHub Actions / Custom App presets for PR-style app attribution
- DOM attributes: className plus data-event-scope and data-event-type for Phase 4 filtering work
Layout: story-local CSS reserves a fixed-width left-of-rail gutter so toggling actorSize doesn't shift the timeline horizontally. Mirrors the Rails ViewComponents .TimelineItem-avatar { left: -72px } treatment without modifying the React component.
janmaarten-a11y
added
skip changeset
|
janmaarten-a11y
had a problem deploying
to
github-pages
GitHub Actions
Failure
|
🤖 Lint and formatting issues have been automatically fixed and committed to this PR. |
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
The build:components.json script only resolves story ids ending in '--default' (in *.stories.tsx), '-features--*' (in *.features.stories.tsx), or '-examples--*' (in *.examples.stories.tsx). The 'components-timeline--playground' id doesn't match any of those patterns and was throwing 'No story named Playground found in Timeline.features.stories.tsx', cascading to ~25 CI job failures (build, lint, type-check, sizes, test, examples, vrt, aat, etc.). Playground stories are not registered in docs.json by convention.
actorName control now hides entirely when actorType is 'copilot' (the name is fixed and not editable). For 'bot', the field stays visible but a useArgs decorator auto-syncs its value to 'dependabot' when the user picks the bot type. Users can still edit from there to pick a different bot identity (e.g. 'renovate[bot]').
Previously the decorator only wrote 'dependabot' when switching to bot, leaving 'dependabot' stuck in the field when switching back to user/app/copilot. Now uses a useRef-tracked previous value to detect any actorType change and write the per-type default (monalisa / dependabot / GitHub Actions / Copilot).
When the user clears the actorName field, the resulting <Link> has no accessible text and fails axe's link-name check. Falls back to 'Unknown actor' for empty/whitespace input. Bot/Copilot baked names already cover those types so the fallback only fires for user/app.
|
I went back through the parent issue's checklist and ran the Storybook a11y panel over the Playground. PR is in good shape; all checks are green. What the a11y review found: one real issue — clearing the What I'm deferring: the issue asked for 2–3 preset stories alongside the Playground. I think we're better off waiting. The Playground already covers every variant via controls, and Phase 2/3 are going to need a real reorganization of the Timeline section (subfolders for primitives, individual events, and full timelines). Adding presets to the current flat structure now would just create churn when that IA work happens. Things I noticed for Phase 2:
|
| padding-left: calc(var(--base-size-40) + var(--base-size-16)); | ||
| } | ||
|
|
||
| .LargeActorAvatar { |
There was a problem hiding this comment.
How many of these are truly story-only since they look like fundamental changes? Should we be representing what things actually render as OOTB and file some follow-up issues to better align the rendering to the expected output rather than overriding locally?
There was a problem hiding this comment.
@adierkens 100% agree, and this is the loudest finding of the whole exercise. The fact that we need story-local CSS to fake a left-rail avatar gutter is exactly what tells us the component needs a real avatar slot. Same for the right-controls floats we left out entirely — lots of events have buttons, SHAs, and other content on the right side. The story-local CSS is deliberately scoped to this file to keep the React component untouched in this PR, so the gap stays loud rather than hidden.
That said, they're not strictly new — the large-avatar treatment mirrors how the equivalent Rails ViewComponents already render today, it just never formally made it into the React API. One of the reasons I've kept this PR as a draft is that I've been wondering whether we should land those API changes first, so when this merges it's not story-only. It's a real chicken/egg question and I'd be curious what you think.
I noted these in the wrap-up comment and on the parent issue github/primer#6662, but you're right that we should have actual followup issues. Will be filing these to scope for Phase 2:
- Avatar slot on
Timeline.Item(covers the large-avatar gutter and the Copilot-avatar fallback that the Rails component has already outgrown) - Right-controls slot
- List semantics evaluation
Octiconcleanup +BADGE_VARIANTSdedup from your other comment
There was a problem hiding this comment.
Two a11y fixes from a manual review: 1. Badge icon: dropped aria-label (which was the developer-facing icon-map key like 'git-commit' or 'x-circle') and added aria-hidden='true'. The icon visually reinforces the summary text; announcing it as a separate label is redundant and reads as jargon. 2. Large actor avatar: changed alt='@monalisa' to alt=''. The actor name is already conveyed by the link immediately after, so the avatar is decorative. The small-avatar branch already used alt='' correctly; large now matches.
Storybook removes the actorName arg entirely (not just the control) when the conditional argType (if neq copilot) hides it. The render then crashed on args.actorName.trim(). Falls back to 'Unknown actor' when actorName is undefined; the resolvedActorName already prefers the baked Copilot name in that case anyway.
Per review feedback: className is just a DOM passthrough that nobody experimenting in a playground is likely to use. The data-* attrs in the same group have semantic purpose for the planned filtering work; className didn't carry its weight.
2 participants
Refs github/primer#6654
Closes github/primer#6662
Phase 1 of the Timeline redesign. Replaces the existing
Components/Timeline → Playgroundstory with a recreation of the Figma "Custom event" component (Primer-Web library, node46191-13560).This is intentionally story-only — no public API changes. The goal is to nail down the prop surface, layout patterns, and naming conventions before Phase 2 introduces named PR / Issue / Dependabot events. The new playground composes existing public primitives only (
Timeline,Timeline.Item,Timeline.Badge,Timeline.Body,Avatar,Link,RelativeTime) and introduces adata-*convention onTimeline.Item(data-event-scope,data-event-type,data-actor-type) that Phase 4 filtering work will build on.Changelog
New
Components/Timelinerecreating the Figma "Custom event" component. Covers 32 badge octicons, all 9TimelineBadgeVariantcolors, 5 timestamp presets (3 relative + 2 absolute), paired GitHub Actions / Custom App attribution, and conditional controls (avatar URL only whenactorType: 'user', app fields only whenviaAppis on, custom-app fields only whenappPreset: 'Custom App').data-*filtering convention on the renderedTimeline.Item:data-event-scope,data-event-type,data-actor-type. No visual effect today; reserved for Phase 4 filtering and the planned summary-events rollup.actorSizedoesn't shift the timeline horizontally. Mirrors the Rails ViewComponents.TimelineItem-avatar { position: absolute; left: -72px }treatment without modifying the React component.Changed
Components/Timeline → Playground(the simpleclipSidebar+condensedtoggle demo) with the new Custom Event playground. The replaced behaviour stays demonstrated inComponents/Timeline/Features(ClipSidebar,ClipSidebarStart,ClipSidebarEnd,CondensedItems).Rollout strategy
@primer/reactpackage surface is unchanged.skip changesetlabel applied.Testing & Reviewing
Open the new playground at
Components/Timeline → Playgroundand try these:actorTypethroughuser / bot / app / copilot. TheactorNamefield auto-fills with a sensible default for each type (monalisa/dependabot/GitHub Actions/Copilot) and stays editable, except forcopilotwhere it's hidden because the name is fixed.actorAvatarSrconly appears foruser.actorSizebetweensmallandlarge. The timeline shouldn't shift horizontally; only the avatar position changes (inline 20px vs 40px in the reserved left gutter).eventTimestampthrough all 5:Relative (now)→just nowRelative (recent day)→yesterday/X days ago(live-updating via<RelativeTime>)Relative (3 weeks)→3 weeks ago(live-updating via<RelativeTime>)Absolute (today)→Today h:mm AM/PM TZ(Intl.DateTimeFormat)Absolute (full timestamp)→Mon DD, h:mm AM/PM TZ(Intl.DateTimeFormat)viaAppattribution — toggle on, switch betweenGitHub ActionsandCustom Apppresets. Custom App reveals editable name + avatar URL fields. Heads up: this slot is PR-specific in real GitHub usage, which we'll revisit in Phase 2.showNote— adds a second muted line below the summary. Covers cited-reason patterns (Dependabot dismissal reason, lock reason, merge-queue "due to..." text).data-event-scope,data-event-type,data-actor-type, andclassNameapply to the renderedTimeline.Item.A11y was reviewed via the Storybook a11y panel. One real
link-nameviolation (emptyactorNameproduced an empty link) was caught and fixed with a fallback. Axe also throws intermittentincompletecolor-contrast notes on user-typed input — that's axe being unable to predict the final color from dynamic text, not a real issue. The Primer muted tokens are used correctly throughout.The longer-form description (Figma reference,
data-*convention, Phase 1 limitations) lives on the story's Docs tab. The Controls panel is intentionally compact since most controls explain themselves through their label and immediate visual feedback.Things this PR doesn't tackle
A few items from the parent issue we decided to defer, plus some bigger gaps that surfaced while building. None of these are blockers for this PR.
Deferred from the issue's scope:
condensedcontrol. Only used on 2nd+ rows of a commit group in real timelines, so it isn't really a custom-event property — and toggling it withactorSize: 'large'looks wrong. Phase 2's named Commit-group event will apply it automatically. The prop is still public onTimeline.Itemand shown inTimeline.features.stories.tsx#CondensedItems.subContent(cross-reference cards, etc.). No card primitive exists yet in Primer React. Adding one would either over-engineer the playground or do work that Phase 2's named events should own.Follow-ups, gaps surfaced during this work, and what we decided to defer are tracked on the parent issue: github/primer#6662.
Merge checklist
Playgroundexport, visible on the Docs tab.Date.now()captured via lazyuseStatefor stability across re-renders.@primer/reactpackage changes; nothing dotcom's Timeline depends on.