Skip to content

[Brand refactor] Testimonial fixes - #1439

Merged
rezrah merged 11 commits into
mainfrom
danielguillan/brand-refactor-testimonial-fixes
Aug 10, 2026
Merged

[Brand refactor] Testimonial fixes#1439
rezrah merged 11 commits into
mainfrom
danielguillan/brand-refactor-testimonial-fixes

Conversation

@danielguillan

@danielguillan danielguillan commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Resolves #1436

Updates the expressive Testimonial variant to match the latest designs across desktop, tablet, and mobile.

🔗 Desktop story

List of notable changes

  • Updates expressive quote typography, responsive sizing, line height, spacing, and attribution styling.
  • Uses a 744px quote column, 120px desktop gap, and 312px attribution column at the large breakpoint.
  • Keeps one semantic link in quote → attribution → link DOM order, placing it in the quote column on desktop and after attribution on tablet/mobile.

Note

The requested 88px top and bottom section padding is intentionally not owned by Testimonial. Vertical section spacing remains the responsibility of the consuming Section or Box composition, which should apply --base-size-88 where required.

What should reviewers focus on?

  • Verify the responsive composition and spacing against the linked Figma design at desktop, tablet, and mobile widths.
  • Confirm the link appears in the desktop quote column but follows attribution on tablet/mobile without duplicate interactive content.
  • Check quote typography, attribution width, divider spacing, blinker color, and light/dark rendering.
  • Confirm subtle, default, and minimal Testimonial variants retain their existing behavior.

Steps to test

  1. Review the desktop story.
  2. Review the tablet story.
  3. Review the mobile story.
  4. Review the dark mode story.
  5. Confirm the intentional visual snapshot differences match the approved design.

Supporting resources

Contributor checklist

  • All new and existing CI checks pass
  • Tests prove that the feature works and preserve non-expressive behavior
  • No coverage regressions; Testimonial branch coverage increased
  • UI changes include updated desktop, tablet, mobile, and dark visual snapshots
  • All developer debugging and non-functional logging has been removed
  • Related issues are referenced in the PR description

Reviewer checklist

  • Check that pull request and proposed changes adhere to our contribution guidelines and code of conduct
  • Check that tests prove the feature works and covers both happy and unhappy paths
  • Check that there aren't other open Pull Requests for the same update/change

Screenshots

Before After
Expressive Testimonial before the responsive design fixes Expressive Testimonial after the responsive design fixes

@changeset-bot

changeset-bot Bot commented Aug 5, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 32dfcec

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@primer/react-brand Patch
@primer/brand-docs Patch
@primer/brand-css Patch
@primer/brand-primitives Patch
@primer/brand-e2e Patch
@primer/brand-fonts Patch
@primer/brand-mcp Patch
@primer/brand-config Patch
@primer/brand-storybook Patch

Not sure what this means? Click here to learn what changesets are.

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

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🟢 No design token changes found

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🟢 Bundle size report

CheckMainBranchChange
UMD — full bundle (JS)101.76 kB101.97 kB⬆️ +214 B (+0.2%)
UMD — full bundle (CSS)67.61 kB67.96 kB⬆️ +359 B (+0.5%)
ESM — full bundle (JS + CSS)1.59 MB1.59 MB⬆️ +2.18 kB (+0.1%)
ESM — tree-shaken simple (Button)70.85 kB71.17 kB⬆️ +333 B (+0.5%)
ESM — tree-shaken complex (ActionMenu)79.81 kB80.13 kB⬆️ +333 B (+0.4%)

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🟢 Unit test coverage changes found

Unit test coverage has been updated through this PR.

Changes: 0 new tests, 0 removed tests, 1 improved, 0 decreased

Component/Hook Statements Functions Branches Change
Testimonial 100.0% 100.0% 90.0% 92.6% +2.6%

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

⚠️ Visual differences found

Our visual comparison tests found UI differences.

Please review the differences by using the test artifacts to ensure that the changes were intentional.

Artifacts can be downloaded and reviewed locally.

Download links are available at the bottom of the workflow summary screen.

Example:

artifacts section of workflow run

If the changes are expected, please run npm run test:visual:update to replace the previous fixtures.

Review visual differences

@danielguillan
danielguillan marked this pull request as ready for review August 6, 2026 07:33
Copilot AI lite review requested due to automatic review settings August 6, 2026 07:33
@danielguillan
danielguillan requested a review from a team as a code owner August 6, 2026 07:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates the Testimonial component’s expressive variant to match the BrandX v1.1 responsive design, including typography/layout changes and ensuring a single semantic link is rendered in the intended DOM order while being visually positioned per breakpoint.

Changes:

  • Refactors expressive layout/styling (grid at large breakpoint, updated spacing/typography, link positioning behavior).
  • Adds dedicated Storybook stories for expressive tablet/mobile and updates story wrappers for fullscreen layout/padding.
  • Expands unit + visual coverage to lock in the new responsive behavior (including new tablet/mobile visual snapshots).
Show a summary per file
File Description
.changeset/fix-responsive-expressive-testimonial.md Adds a patch changeset describing the expressive Testimonial update.
packages/react/src/Testimonial/Testimonial.features.stories.tsx Adds expressive tablet/mobile stories and switches expressive examples to a shared padded container with fullscreen layout.
packages/react/src/Testimonial/Testimonial.module.css Implements the updated expressive responsive layout (stack → grid), typography, spacing, and link placement styles.
packages/react/src/Testimonial/Testimonial.module.css.d.ts Updates generated CSS module typings for new/added class names.
packages/react/src/Testimonial/Testimonial.stories.module.css Introduces an expressiveContainer wrapper for consistent max-width + responsive padding in stories.
packages/react/src/Testimonial/Testimonial.stories.module.css.d.ts Updates generated story CSS module typings for the new container class.
packages/react/src/Testimonial/Testimonial.test.tsx Updates/expands unit tests to assert a single link renders after attribution for expressive while preserving non-expressive behavior.
packages/react/src/Testimonial/Testimonial.tsx Adjusts expressive DOM placement and adds a conditional class to support grid row behavior when a link exists.
packages/react/src/Testimonial/Testimonial.visual.spec.ts Adds new tablet and mobile visual regression tests for expressive stories.

Review details

  • Files reviewed: 7/15 changed files
  • Comments generated: 0
  • Review effort level: Lite

@danielguillan
danielguillan requested a review from rezrah August 6, 2026 10:12

@rezrah rezrah left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Code changes LGTM. Deferring to @jesussandreas or Site on design accuracy and sign off.

@jesussandreas

Copy link
Copy Markdown
Collaborator

@rezrah looks great,

i have 2 comments:

  1. the avatar on mobile/tablet - lets reduce it to 40px / i realized there's no references of this in figma (this is on me and apologies for that).

  2. on the testimonial on tablet, the layout grid should match the inner grid content, just calling that out since it seemed that the current layout was extending too much? I just want to ensure it matches the layout content width with the others

Screenshot 2026-08-06 at 8 26 07 PM

@jesussandreas

Copy link
Copy Markdown
Collaborator

@danielguillan @rezrah one last question, for the spacing - are we baking those inside each component or how are we handling those?

i see currently is set to 48px~ or so. thats okay just wondering if we want a fix value here or have it dynamic?

@danielguillan
danielguillan marked this pull request as draft August 10, 2026 07:45
@rezrah

rezrah commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

@jesussandreas I'm stepping in for Dani on this PR as he's now OOO. I'm coming in with almost zero context, so bear with me. I've so far focussed on:

  • Matching spacing to the Figma file
  • Improving text quality

Assuming Dani's previous changes cover everything else.

one last question, for the spacing - are we baking those inside each component or how are we handling those?

Testimonial doesn't usually include it's own padding. We also generally don't add margins or internal outer paddings to components unless they are:

  • Part of the core design
  • Too easy to get wrong in the wider layout

In this case, i would say the latter definitely matches. So I've added internal padding to match designs

  • 20px on narrow
  • 36px on tablet with 618px max width and centered
  • 0 at widest but with 1176 max width and centered

If these look wrong though, can we update the Figma layouts please.

Another thing i changed was adding AA to the text. Something looked really off about Storybook, and I'm pretty sure it was just the lack of AA that is enabled in Figma.

Before After
Screenshot 2026-08-10 at 15 02 43 Screenshot 2026-08-10 at 15 02 28

@jesussandreas we need to land this pretty soon to unblock the release. Can you please ✔️ or guide me on next steps / remaining issues.

@rezrah
rezrah marked this pull request as ready for review August 10, 2026 14:41

@jesussandreas jesussandreas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

amazing

@rezrah
rezrah merged commit bca51bc into main Aug 10, 2026
17 checks passed
@rezrah
rezrah deleted the danielguillan/brand-refactor-testimonial-fixes branch August 10, 2026 17:42
@primer primer Bot mentioned this pull request Aug 10, 2026
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.

🐛 [BUG] - Testimonial brand-refactor design mismatch

4 participants