[IRN-5969][BpkPanel] panel keyline #3869

jimmycook · 2025-07-03T11:38:15Z

keyline changes
typescript

We're adding a keyline prop to the panel component, on by default, that can be disabled.3

Also moving the component to typescript.

Remember to include the following changes:

skyscanner-backpack-bot · 2025-07-03T11:43:41Z

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

jimmycook · 2025-07-03T11:45:14Z

packages/bpk-component-panel/src/BpkPanel.tsx

+  padded?: boolean,
+  fullWidth?: boolean,
+  className?: string | null,
+  keyline?: boolean,


Core of the change, this prop, on by default

jimmycook · 2025-07-03T11:45:32Z

packages/bpk-component-panel/src/BpkPanel.tsx


  if (padded) {
    classNames.push(getClassName('bpk-panel--padded'));
  }
  if (fullWidth) {
    classNames.push(getClassName('bpk-panel--full-width'));
  }
+  if (keyline) {


Decide which keyline class to use based on full width or not

skyscanner-backpack-bot · 2025-07-03T12:01:42Z

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

skyscanner-backpack-bot · 2025-07-03T12:31:50Z

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

skyscanner-backpack-bot · 2025-07-03T12:34:06Z

	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 c2a36d7

skyscanner-backpack-bot · 2025-07-03T14:35:21Z

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

Copilot

Pull Request Overview

Adds a new keyline prop (enabled by default, can be disabled) to the BpkPanel component and converts it to TypeScript.

Introduces two new SCSS mixins for keyline support.
Updates BpkPanel.tsx to define and use the keyline prop with a default of true.
Updates tests, snapshots, CSS modules, and Storybook examples to cover the new behavior.

Reviewed Changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/bpk-mixins/_panels.scss	Adds `bpk-panel--keyline` and `bpk-panel--full-width-keyline` mixins
packages/bpk-component-panel/src/BpkPanel.tsx	Ports to TypeScript, defines `Props`, and implements `keyline` logic
packages/bpk-component-panel/src/BpkPanel.module.scss	Adds new CSS module classes for keyline variants
packages/bpk-component-panel/src/snapshots/BpkPanel-test.tsx.snap	Updates snapshots to include keyline classes
packages/bpk-component-panel/src/BpkPanel-test.tsx	Adds tests for disabling keyline in both normal and full-width modes
examples/bpk-component-panel/stories.js	Exposes a `NoKeyline` Storybook story
examples/bpk-component-panel/examples.js	Adds `NoKeylineExample` to the live examples

Comments suppressed due to low confidence (1)

packages/bpk-mixins/_panels.scss:92

The @mixin bpk-panel--full-width-keyline definition is missing a closing }. Without it, the stylesheet will fail to compile.

@mixin bpk-panel--full-width-keyline {

skyscanner-backpack-bot · 2025-07-04T06:10:21Z

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

Faye-Xiao · 2025-07-04T06:16:17Z

examples/bpk-component-panel/examples.js

@@ -58,4 +68,5 @@ export {
  WithoutPaddingExample,
  FullWidthExample,
  MixedExample,
+  NoKeylineExample,
 };


Thank you for converting the related code to TS!
Could you also convert the example and stories files to TS? 🙇‍♀️

Done in next commit

Faye-Xiao · 2025-07-04T06:30:05Z

examples/bpk-component-panel/examples.js

+    consectetur, dolor nec vulputate vehicula, ex metus mattis ante, non dictum
+    mi ante eu arcu.
+  </BpkPanel>
+);


Hi @jimmycook could you add a container with a non-white background for this BpkPanel as it's a little hard to see its visual on a white background?

How it looks like on your storybook case

visual spec

Faye-Xiao · 2025-07-04T06:38:03Z

packages/bpk-component-panel/src/BpkPanel-test.tsx

@@ -54,6 +54,28 @@ describe('BpkPanel', () => {
    expect(asFragment()).toMatchSnapshot();
  });

+  it('should render with no keyline when attribute is false', () => {


Nit: 'should render BpkPanel without keyline when keyline attribute is false'

Faye-Xiao · 2025-07-04T06:41:06Z

packages/bpk-component-panel/src/BpkPanel-test.tsx

+        dis parturient montes, nascetur ridiculus mus.
+      </BpkPanel>,
+    );
+    expect(asFragment()).toMatchSnapshot();


We do not recommend using snapshot testing. Could you use jest expect instead? 🙇‍♀️

TLDR: Avoid using snapshots for testing components where possible

Removed all of these

Faye-Xiao · 2025-07-04T06:41:31Z

packages/bpk-component-panel/src/BpkPanel-test.tsx

+        dis parturient montes, nascetur ridiculus mus.
+      </BpkPanel>,
+    );
+    expect(asFragment()).toMatchSnapshot();


Same as comment r2184540978

Faye-Xiao · 2025-07-04T06:48:17Z

packages/bpk-component-panel/src/BpkPanel.tsx

+  fullWidth = false,
+  keyline = true,
+  padded = true,
+  ...rest


It looks like the rest type definition is missing in Props .

Could you add a type for it?

e.g.

export type Props = { ... [rest: string]: any; ... }

skyscanner-backpack-bot · 2025-07-07T12:20:17Z

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

Faye-Xiao

LGTM. 🚀

Faye-Xiao · 2025-07-08T02:52:17Z

Approved the Percy failure https://percy.io/09e69e44/web/backpack/builds/41387263?utm_campaign=09e69e44&utm_content=backpack&utm_source=github_status_public as that's what we expected.

jimmycook added 2 commits July 3, 2025 11:45

keyline changes

d5f5aad

typescript

26cbf29

jimmycook added the minor Non breaking change label Jul 3, 2025

jimmycook commented Jul 3, 2025

View reviewed changes

cleanup

6028dc7

update snapshot

e206806

fix comment docs

18c1a26

Faye-Xiao requested a review from Copilot July 4, 2025 06:08

Merge branch 'main' into IRN-5969-panel-keyline

a3c7ac5

Copilot AI reviewed Jul 4, 2025

View reviewed changes

Faye-Xiao reviewed Jul 4, 2025

View reviewed changes

jimmycook and others added 2 commits July 7, 2025 13:13

action PR comments

c9ab31a

Merge branch 'main' into IRN-5969-panel-keyline

c2a36d7

Faye-Xiao approved these changes Jul 8, 2025

View reviewed changes

Faye-Xiao merged commit a3d166c into main Jul 8, 2025
10 checks passed

Faye-Xiao deleted the IRN-5969-panel-keyline branch July 8, 2025 02:52

Faye-Xiao added V38 Planned this change in V38 version and removed V38 Planned this change in V38 version labels Jul 9, 2025

[IRN-5969][BpkPanel] panel keyline #3869

[IRN-5969][BpkPanel] panel keyline #3869

Uh oh!

Conversation

jimmycook commented Jul 3, 2025

Uh oh!

skyscanner-backpack-bot bot commented Jul 3, 2025

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 Jul 3, 2025

Uh oh!

skyscanner-backpack-bot bot commented Jul 3, 2025

Uh oh!

skyscanner-backpack-bot bot commented Jul 3, 2025 • 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 Jul 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

skyscanner-backpack-bot bot commented Jul 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Faye-Xiao Jul 4, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Faye-Xiao Jul 4, 2025 • 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!

skyscanner-backpack-bot bot commented Jul 7, 2025

Uh oh!

Faye-Xiao left a comment

Choose a reason for hiding this comment

Uh oh!

Faye-Xiao commented Jul 8, 2025

Uh oh!

Uh oh!

Uh oh!

skyscanner-backpack-bot bot commented Jul 3, 2025 •

edited

Loading

Faye-Xiao Jul 4, 2025 •

edited

Loading

Faye-Xiao Jul 4, 2025 •

edited

Loading