[CLOV-1337][BpkButton] Add leadingIcon and trailingIcon props by RichardSyq · Pull Request #4269 · Skyscanner/backpack

Richard-Shen (RichardSyq) · 2026-03-10T10:40:01Z

Summary

Adds leadingIcon and trailingIcon optional props to BpkButtonV2, providing dedicated icon slots before and after the button label.

Previously, icons had to be passed via children with manual alignment using withButtonAlignment(). The new props offer a cleaner API with automatic flexbox alignment, consistent spacing, and no wrapper needed.

Changes

common-types.tsx — added leadingIcon?: ReactNode and trailingIcon?: ReactNode to Props
BpkButton.tsx — renders icon wrapper spans, applies bpk-button--has-icon class when icons are present
BpkButton.module.scss — flexbox layout with gap for spacing; text-decoration: none on icon wrappers to prevent underline bleeding on link buttons
BpkButton.figma.tsx — updated Code Connect to use figma.boolean for leadingIcon, trailingIcon and iconOnly
BpkButton-test.tsx — DOM assertion tests for icon prop variants
examples/ — all button type stories updated to use leadingIcon/trailingIcon props; removed old children-based icon pattern

Remember to include the following changes:

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

skyscanner-backpack-bot · 2026-03-10T10:47:23Z

Visit https://backpack.github.io/storybook-prs/4269 to see this build running in a browser.

Copilot

Pull request overview

Adds support for optional leading and trailing icons to BpkButtonV2, including styling and Storybook/Figma examples, plus updated tests to cover the new API.

Changes:

Extend BpkButtonV2 props with leadingIcon and trailingIcon, and render them with new BEM element wrappers.
Add styling for icon layout (--has-icon, __leading-icon, __trailing-icon) and full-width alignment.
Update accessibility/unit tests and add Storybook + Figma Code Connect examples for the new icon props.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
packages/bpk-component-button/src/BpkButtonV2/common-types.tsx	Adds `leadingIcon`/`trailingIcon` to the component props type.
packages/bpk-component-button/src/BpkButtonV2/BpkButton.tsx	Renders optional leading/trailing icons and applies a `--has-icon` modifier.
packages/bpk-component-button/src/BpkButtonV2/BpkButton.module.scss	Adds layout rules for icon-bearing buttons and icon wrappers.
packages/bpk-component-button/src/BpkButtonV2/accessibility-test.tsx	Adds axe coverage for leading/trailing icon configurations.
packages/bpk-component-button/src/BpkButtonV2/BpkButton-test.tsx	Adds unit tests for icon rendering and class application (incl. snapshots).
packages/bpk-component-button/src/BpkButtonV2/BpkButton.figma.tsx	Updates Code Connect props to use `leadingIcon`/`trailingIcon`.
examples/bpk-component-button/stories.tsx	Adds new Storybook stories/visual test entries for icon props.
examples/bpk-component-button/examples.tsx	Adds Storybook example components that exercise the new icon props.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

packages/bpk-component-button/src/BpkButtonV2/BpkButton-test.tsx

packages/bpk-component-button/src/BpkButtonV2/BpkButton.figma.tsx

Copilot · 2026-03-10T10:47:53Z

packages/bpk-component-button/src/BpkButtonV2/BpkButton.module.scss

+  &--full-width#{&}--has-icon {
+    display: flex;
+    justify-content: center;
+  }


The selector &--full-width#{&}--has-icon works, but the interpolation makes it harder to read/maintain. This can be expressed more clearly as &--full-width&--has-icon (same combined selector) while avoiding string interpolation.

The current version of sass-loader does not support &--full-width&--has-icon

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

skyscanner-backpack-bot · 2026-03-10T15:29:09Z

Visit https://backpack.github.io/storybook-prs/4269 to see this build running in a browser.

skyscanner-backpack-bot · 2026-03-10T15:30:47Z

	Warnings
⚠️	Package source files (e.g. `packages/package-name/src/Component.js`) were updated, but snapshots weren't. Have you checked that the tests still pass?

Browser support

If this is a visual change, make sure you've tested it in multiple browsers.

Generated by 🚫 dangerJS against 4c4af47

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

skyscanner-backpack-bot · 2026-03-11T04:27:58Z

Visit https://backpack.github.io/storybook-prs/4269 to see this build running in a browser.

…railingIcon props Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

skyscanner-backpack-bot · 2026-03-11T07:59:33Z

Visit https://backpack.github.io/storybook-prs/4269 to see this build running in a browser.

- Use figma.boolean for leadingIcon, trailingIcon and iconOnly props - Move leadingIcon/trailingIcon adjacent in props destructuring Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

skyscanner-backpack-bot · 2026-03-11T08:17:49Z

Visit https://backpack.github.io/storybook-prs/4269 to see this build running in a browser.

Richard-Shen (RichardSyq)

Code review

Found 1 issue:

Non-semantic spacing token used for icon-to-text gap — $bpk-spacing-icon-text: .5rem exists in backpack-foundations specifically for icon-to-text spacing and produces the same value as bpk-spacing-md(). Using the size-based function instead of the semantic variable breaks the principle stated in the Constitution and AGENTS.md: "Use semantic color tokens that describe purpose, not appearance" — the same applies to spacing. Replace gap: tokens.bpk-spacing-md() with gap: tokens.$bpk-spacing-icon-text.

backpack/packages/bpk-component-button/src/BpkButtonV2/BpkButton.module.scss

Lines 113 to 118 in 93a2566

    
           &--has-icon { 
        
             display: inline-flex; 
        
             align-items: center; 
        
             gap: tokens.bpk-spacing-md(); 
        
           }

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-11T08:28:58Z

packages/bpk-component-button/src/BpkButtonV2/BpkButton.figma.tsx

+      iconOnly: figma.boolean('Icon only')
    },
-    example: ({ iconOnly, isDisabled, label, leftContent, rightContent, size, style }) => (
-        <BpkButton type={style} size={size} disabled={isDisabled} iconOnly={iconOnly}>
-          {leftContent}
+    example: ({ iconOnly, isDisabled, label, leadingIcon, size, style, trailingIcon }) => (
+        <BpkButton
+          type={style}
+          size={size}
+          disabled={isDisabled}
+          iconOnly={iconOnly}
+          leadingIcon={leadingIcon}
+          trailingIcon={trailingIcon}
+        >
          {label}
-          {rightContent}
        </BpkButton>


In the Code Connect example, iconOnly is passed through but the button still renders {label} as children and doesn’t provide an accessible name. When iconOnly is true the rendered output will be an “icon-only styled” button that still contains text, and (if you switch to rendering only an icon) it would also need aria-label (e.g., derived from label). Consider branching the example so that iconOnly=true renders an icon as children and sets aria-label={label}, and also avoids passing leadingIcon/trailingIcon in that mode.

Fixed. The example is now branched on iconOnly:

When iconOnly=true: renders as children, sets aria-label={label}, and omits leadingIcon/trailingIcon

When iconOnly=false: renders {label} as children with leadingIcon/trailingIcon passed through as normal

Richard-Shen (RichardSyq) · 2026-03-11T08:35:15Z

Code review

Found 1 issue:

Non-semantic spacing token used for icon-to-text gap — $bpk-spacing-icon-text: .5rem exists in backpack-foundations specifically for icon-to-text spacing and produces the same value as bpk-spacing-md(). Using the size-based function instead of the semantic variable breaks the principle stated in the Constitution and AGENTS.md: "Use semantic color tokens that describe purpose, not appearance" — the same applies to spacing. Replace gap: tokens.bpk-spacing-md() with gap: tokens.$bpk-spacing-icon-text.

backpack/packages/bpk-component-button/src/BpkButtonV2/BpkButton.module.scss

Lines 113 to 118 in 93a2566

&--has-icon {

display: inline-flex;

align-items: center;

gap: tokens.bpk-spacing-md();

}

🤖 Generated with Claude Code

If this code review was useful, please react with 👍. Otherwise, react with 👎.

However, no components under packages/ use $bpk-spacing-icon-text, so I think we should continue using bpk-spacing-md().

…Icon/trailingIcon - README: add leadingIcon/trailingIcon usage examples - button-link-type.md: update With Icons section to show new props as recommended approach; deprecate icon-as-children-sibling pattern; restore Custom Text Styling with BpkText with migration note - BpkButton.figma.tsx: branch iconOnly example to render icon as children with aria-label, avoiding invalid leadingIcon/trailingIcon usage in icon-only mode Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…rops

skyscanner-backpack-bot · 2026-03-11T09:05:49Z

Visit https://backpack.github.io/storybook-prs/4269 to see this build running in a browser.

Gert-Jan Vercauteren (gert-janvercauteren)

Implementation looks alright! Just a catch on hovering on the Link type button. (This is somewhat pre-existing, but would be good to resolve right here and now :D )

packages/bpk-component-button/docs/button-link-type.md

packages/bpk-component-button/src/BpkButtonV2/BpkButton.tsx

GC Zhu (gc-skyscanner) · 2026-03-12T03:57:50Z

I assumed it is an issue in the case, could you help to fix it?

GC Zhu (gc-skyscanner) · 2026-03-12T06:24:53Z

packages/bpk-component-button/src/BpkButtonV2/BpkButton.tsx

        rel={rel}
        {...rest}
      >
+        {leadingIconEl}


FYI https://github.com/Skyscanner/backpack/pull/4275/changes#r2922548004

GC Zhu (gc-skyscanner) · 2026-03-12T06:26:02Z

examples/bpk-component-button/examples.tsx

+      &nbsp;
+      <Wrapped
+        size={SIZE_TYPES.large}
+        leadingIcon={<SmallLightningIcon />}


Nit: Maybe we should use large icon here?

Thanks you reminder, I have modified this part.

GC Zhu (gc-skyscanner) · 2026-03-12T06:39:52Z

packages/bpk-component-button/src/BpkButtonV2/BpkButton.tsx

@@ -35,16 +35,19 @@ export const BpkButtonV2 = ({
  href = null,
  iconOnly = false,


Sorry for being a bit annoying, do we need to handle the case where iconOnly=true but leadingIcon/trailingIcon are also passed? I tried it and it shows three icons. Is this intended?

That's a good question! I'll consider this situation.

I believe the two newly added props should not affect the case where iconOnly=true. I've configured it so that leading icons and trailing icons are not rendered when iconOnly=true.

Is there a way to add typesafety where if iconOnly = true, then leading/trailing prop are not allowed?

Gert-Jan Vercauteren (@gert-janvercauteren) I added this condition; do you think it's suitable as a typesafety?

https://github.com/Skyscanner/backpack/pull/4269/changes#diff-2689f3f4f8ca26010db7794aa0cf720fd797f59dc63380171b13a29377e9d2b1R78

- Use LargeLightningIcon for large button examples instead of small - Fix link button hover animation to trigger from root button hover (not just text span) using CSS variables - Ignore leadingIcon/trailingIcon when iconOnly=true to prevent three-icon rendering Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

skyscanner-backpack-bot · 2026-03-12T08:27:47Z

Visit https://backpack.github.io/storybook-prs/4269 to see this build running in a browser.

…er selectors - Replace CSS variable approach with pure descendant hover selectors to avoid overriding dark mode underline styles - Only apply hover animation override when icons are present (--has-icon) - Remove link/linkOnDark anchor tag href from AnchorTagsExample to avoid pre-existing vertical centering issue with <a> tags Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Richard-Shen (RichardSyq) · 2026-03-12T09:18:38Z

I assumed it is an issue in the case, could you help to fix it?

This is a pre-existing issue with large link buttons rendered as anchor tags ( via href). The base bpk-button mixin uses display: inline-block, which doesn't vertically center content in elements the way does natively.

I've solved it by removing href="#".

skyscanner-backpack-bot · 2026-03-12T09:25:04Z

Visit https://backpack.github.io/storybook-prs/4269 to see this build running in a browser.

…rops

skyscanner-backpack-bot · 2026-03-12T10:05:37Z

Visit https://backpack.github.io/storybook-prs/4269 to see this build running in a browser.

Gert-Jan Vercauteren (gert-janvercauteren)

One last thing and then this feels good to go!

Gert-Jan Vercauteren (gert-janvercauteren) · 2026-03-12T13:16:43Z

packages/bpk-component-button/src/BpkButtonV2/BpkButton.tsx

        rel={rel}
        {...rest}
      >
+        {leadingIconEl}


We should consider treating content for both link and button the same, should we extract it out and then put in? (Considering there's now conditional rendering for button but not for

[CLOV-1337][BpkButton] Add leadingIcon and trailingIcon props

82f8d23

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings March 10, 2026 10:40

Copilot started reviewing on behalf of Richard-Shen (RichardSyq) March 10, 2026 10:40 View session

Richard-Shen (RichardSyq) self-assigned this Mar 10, 2026

Richard-Shen (RichardSyq) added minor Non breaking change ai: claude labels Mar 10, 2026

Copilot AI reviewed Mar 10, 2026

View reviewed changes

[CLOV-1337][BpkButton] Add snapshots and link-with-icon story

15cf402

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Richard Shen and others added 3 commits March 11, 2026 11:15

[CLOV-1337][BpkButton] Replace snapshot tests with DOM assertions

92a2064

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

[CLOV-1337] Restore package-lock.json to main

e517f79

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

[CLOV-1337] Restore package-lock.json to origin/main

ee4d811

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

[CLOV-1337][BpkButton] Update Storybook examples to use leadingIcon/t…

4786b2f

…railingIcon props Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

[CLOV-1337][BpkButton] Address Copilot review comments

93a2566

- Use figma.boolean for leadingIcon, trailingIcon and iconOnly props - Move leadingIcon/trailingIcon adjacent in props destructuring Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Richard-Shen (RichardSyq) commented Mar 11, 2026

View reviewed changes

Richard-Shen (RichardSyq) requested a review from Copilot March 11, 2026 08:24

Copilot started reviewing on behalf of Richard-Shen (RichardSyq) March 11, 2026 08:25 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

Richard Shen and others added 2 commits March 11, 2026 16:51

Merge branch 'main' into CLOV-1337_bpk_button_leading_trailing_icon_p…

179a813

…rops

Gert-Jan Vercauteren (gert-janvercauteren) reviewed Mar 12, 2026

View reviewed changes

packages/bpk-component-button/docs/button-link-type.md Show resolved Hide resolved

packages/bpk-component-button/src/BpkButtonV2/BpkButton.tsx Outdated Show resolved Hide resolved

packages/bpk-component-button/src/BpkButtonV2/BpkButton.tsx Show resolved Hide resolved

GC Zhu (gc-skyscanner) reviewed Mar 12, 2026

View reviewed changes

GC Zhu (gc-skyscanner) mentioned this pull request Mar 12, 2026

[BpkButton]Add loading prop for BpkButton #4275

Open

5 tasks

GC Zhu (gc-skyscanner) reviewed Mar 12, 2026

View reviewed changes

This comment was marked as duplicate.

Sign in to view

Richard Shen and others added 3 commits March 12, 2026 17:32

Remove comment

84c3ec2

Fix issue

2bd407d

Merge branch 'main' into CLOV-1337_bpk_button_leading_trailing_icon_p…

4c4af47

…rops

Richard-Shen (RichardSyq) requested review from GC Zhu (gc-skyscanner) and Gert-Jan Vercauteren (gert-janvercauteren) March 12, 2026 10:15

Gert-Jan Vercauteren (gert-janvercauteren) reviewed Mar 12, 2026

View reviewed changes

	&--has-icon {
	display: inline-flex;
	align-items: center;
	gap: tokens.bpk-spacing-md();
	}

		@@ -35,16 +35,19 @@ export const BpkButtonV2 = ({
		href = null,
		iconOnly = false,

Conversation

Richard-Shen (RichardSyq) commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Uh oh!

skyscanner-backpack-bot bot commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skyscanner-backpack-bot bot commented Mar 10, 2026

Uh oh!

skyscanner-backpack-bot bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Browser support

Uh oh!

skyscanner-backpack-bot bot commented Mar 11, 2026

Uh oh!

skyscanner-backpack-bot bot commented Mar 11, 2026

Uh oh!

skyscanner-backpack-bot bot commented Mar 11, 2026

Uh oh!

Richard-Shen (RichardSyq) left a comment

Choose a reason for hiding this comment

Code review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Richard-Shen (RichardSyq) commented Mar 11, 2026

Code review

Uh oh!

skyscanner-backpack-bot bot commented Mar 11, 2026

Uh oh!

Gert-Jan Vercauteren (gert-janvercauteren) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GC Zhu (gc-skyscanner) commented Mar 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GC Zhu (gc-skyscanner) Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skyscanner-backpack-bot bot commented Mar 12, 2026

Uh oh!

Richard-Shen (RichardSyq) commented Mar 12, 2026

Richard-Shen (RichardSyq) commented Mar 10, 2026 •

edited

Loading

skyscanner-backpack-bot bot commented Mar 10, 2026 •

edited

Loading

GC Zhu (gc-skyscanner) Mar 12, 2026 •

edited

Loading