feat(ui-banner): add Banner component with locally vendored design tokens (testing only) - #2722
Draft
adamlobler wants to merge 2 commits into
Draft
adamlobler wants to merge 2 commits into
adamlobler wants to merge 2 commits into
Conversation
…mponent with locally vendored design tokens
Add Banner, a proactive promotional/announcement component distinct from
Alert's reactive system-status messaging: no live-region semantics, a
labelled section landmark, an icon, optional title, optional close button,
and optional CTA actions in two color treatments (plum, sky).
Banner's design tokens don't exist in the pinned instructure-design-tokens
v1.5.0 release, so this branch vendors that release's token source locally
into design-tokens/ and points ui-scripts at it via a link: dependency
instead of bumping to a newer release. This keeps the new component's
tokens independent of newer, not-yet-reviewed upstream token work. See the
PR description and design-tokens/README-VENDORED.md for details and the
path back to the real instructure-design-tokens repo.
New tokens added on top of vendored v1.5.0:
- semantic.color.background.accentMuted.{plum,sky} (all four themes) - a
washed-back surface variant of the existing accent.{plum,sky} family
- tokensStudio/{rebrand,canvas}/component/Banner.json - the component's
full token set (colors, spacing, radii, borders, typography)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI failed with ERR_MODULE_NOT_FOUND: Cannot find package 'glob' imported from design-tokens/src/index.js. The vendored token loader used glob to enumerate tokensStudio/**/*.json, but design-tokens is consumed via a link: dependency rather than a normal package install, so pnpm never installs its declared dependencies - a fresh checkout has no glob for it to resolve. Locally this was papered over with an ad hoc npm install inside design-tokens/, which never got captured in git (node_modules is gitignored, and its incidental package-lock.json shouldn't have been committed either). Replace globSync with Node's built-in recursive readdirSync instead of vendoring glob too - the repo already requires Node >=22.18, well past when that API landed. Verified the regenerated newThemeTokens output is still byte-identical to before this change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
Contributor
Visual regression reportCypress suite: ✅ Passing Visual diff: ✅ No changes.
Accessibility (axe): ✅ No violations. 📊 View full report — click a screenshot's ⚠ badge to see each violation boxed on the image, with the offending element named and contrast failures shown as color swatches. Baselines come from the |
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
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.
This PR is for review and discussion, not merging. It's a proof-of-concept exercise: draft an entirely new InstUI component (
Banner) from a plain-language description, with no Figma source, using only the design tokens already released ininstructure-design-tokensv1.5.0.Why the tokens are vendored into this repo instead of bumped to a newer release:
Bannerhas no token set in the pinned v1.5.0 release. A newer upstream release (v1.8.0) already ships an officialBannertoken set, discovered midway through this exercise — but using it would have meant designing against someone else's already-made decisions instead of deriving a design independently from v1.5.0's existing vocabulary. So this branch:design-tokens/in this repo, and proves that vendoring step is a no-op (regeneratednewThemeTokensoutput is identical to before vendoring)packages/ui-scriptsat that local copy via alink:dependency instead of the realgithub:...#v1.5.0pinBanner's tokens directly in that local copyThis is not how token changes should ship for real. The same additions (see below) could be made directly in the instructure-design-tokens repo, tagged as a real release, and then this repo would just bump its pin — no vendoring, no
link:dependency. That's the path to actually shipping this component; this branch exists to test the component design without needing write access to that repo or waiting on a release.Process
Alert's v2 implementation (withStyleNew, close-button/icon conventions) and researched how other design systems (Polaris, Spectrum, NewsKit, Primer) distinguish a promotional "Banner" from a status "Alert".instructure-design-tokensv1.5.0 actually contains before designing anything — confirmed noBannerset exists, and inventoried the existingbackground.accent.<hue>family as the right non-status semantic home for a promotional color.violet/seaturned out to be reserved for AI theming in v1.5.0,blue/green/red/orangeare the status colors,navyis brand. Landed onplum+sky— the second color was originallyhoney, thenstone, retuned twice after visual review flaggedhoneyas too close towarning's orange.Banner's full component token set againstAlert's file as a structural reference, then registered it in$metadata.json/$themes.json.forwardRef,useStyleNew,useDeterministicId), matching the majority prop-naming convention in the codebase (renderIcon,renderTitle,onDismiss,screenReaderLabel) rather than the specific names that had appeared in the discarded v1.8.0 preview.getComputedStyleoutput in a running docs instance, across all four themes independently, rather than trusting the generated docs table.packages/ui, the docs app, and the regression-test app; added unit tests and a regression page.New tokens
Semantic — one new alias family, added to all four themes (
light,dark,legacyCanvas,legacyCanvasHighContrast):semantic.color.background.accentMuted.plumaccent.plumhue — used forBanner's card surface, distinct per theme so it isn't a fixed alpha overlay.semantic.color.background.accentMuted.skyskyhue.Component (
tokensStudio/{rebrand,canvas}/component/Banner.json, one set per brand family):plumBackground/skyBackgroundaccentMuted.<hue>).plumIconBackground/skyIconBackgroundaccent.<hue>) — distinct from the washed card surface.iconColoricon.onColor).colortext.base).titleColortext.base—text.titledoesn't exist in v1.5.0).borderRadiusborderRadius.xl) — deliberately rounder thanAlert'slg.iconContainerBorderRadiusborderRadius.lg).borderWidth/borderStyle/borderColorpaddingVertical/paddingHorizontaliconGapstackGapactionGapcloseButtonMarginTop/closeButtonMarginEndtitleFontFamily/titleFontSize/titleFontWeight/titleLineHeightcontentFontFamily/contentFontSize/contentFontWeight/contentLineHeightComponent properties
color'plum' | 'sky'Alert's variants — pick whichever reads best against surrounding content.renderIconRenderableBanner. Defaults to a megaphone icon so one is always present; treated as decorative.renderTitleRenderableBanner's landmark for assistive technology.childrenReactNoderenderActionsRenderableButtons).renderCloseButtonLabelRenderableonDismiss() => voidBanner. The component doesn't manage its own visibility.screenReaderLabelstringrenderTitle.elementRef(el: Element | null) => voidmarginSpacingBanner.Test Plan
/#Bannerand check all four themes (canvas,canvas-high-contrast,light,dark) — the two color treatments should read as promotional, not as status colors.Bannernever setsrole="alert"oraria-live, unlikeAlert./regression-test's/bannerpage for the six example states (both colors, dismissible, with actions, screen-reader-label-only).🤖 Generated with Claude Code