[DTO-4949][BpkNavigationBar] Create on-dark variant of BpkNavigationBar & Add Typescript #3110

Iain530 · 2023-12-06T10:54:59Z

Description

Figma link

Added prop barStyle to BpkNavigationBar, BpkNavigationBarButtonLink, BpkNavigationBarIconButton
Added on-dark style
Migrated BpkNavigationBar to typescript

Remember to include the following changes:

Ensure the PR title includes the name of the component you are changing so it's clear in the release notes for consumers of the changes in the version e.g [KOA-123][BpkButton] Updating the colour
README.md (If you have created a new component)
Component README.md
Tests
Storybook examples created/updated
For breaking changes or deprecating components/properties, migration guides added to the description of the PR. If the guide has large changes, consider creating a new Markdown page inside the component's docs folder and link it here

Iain530 · 2023-12-06T11:50:06Z

Build currently failing due to missing dependency, will be fixed after #3109 is merged

github-actions · 2023-12-06T12:17:14Z

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

github-actions · 2023-12-06T12:17:15Z

	Warnings
⚠️	Package source files (e.g. `packages/package-name/src/Component.tsx`) were updated, but type files weren't. Have you checked that no types have changed?

Browser support

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

Generated by 🚫 dangerJS against 255eebc

github-actions · 2023-12-06T12:35:59Z

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

Iain530 · 2023-12-06T12:46:53Z

Percy changes look as expected 👍

packages/bpk-component-navigation-bar/src/themeAttributes.js

anambl · 2023-12-06T15:33:01Z

packages/bpk-component-navigation-bar/src/BpkNavigationBar.js

+  'default': 'default',
+  onDark: 'on-dark',
+};
+export type BarStyle = $Values<BAR_STYLES>


Do you think you could convert it into TS as we are moving away from flow? We've added this bit in the contribution guidelines https://github.com/Skyscanner/backpack/blob/main/CONTRIBUTING.md#typescript but maybe it's not too visible 😢

Yes can do, I will admit I didn't re-read that for this PR so that's on me 🙈

Migrated to typescript, does this mean it will need a major version change or still a minor?

I think it's ok to keep it as minor 🤔 Looking at the changelog, I see we've generally released these as minor.

anambl · 2023-12-06T15:35:06Z

packages/bpk-component-modal/src/__snapshots__/BpkModalInner-test.tsx.snap

Hmm this also means it's going to break snapshots likely for users of navBar, but I guess that's fine, we can add a note maybe in the changelog 🤔

github-actions · 2023-12-08T16:14:21Z

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

packages/bpk-component-navigation-bar/src/BpkNavigationBar-test.tsx

Iain530 · 2023-12-08T16:21:13Z

packages/bpk-component-navigation-bar/src/BpkNavigationBarButtonLink.tsx

 import PropTypes from 'prop-types';

-// TODO: close button is not really only a close button, we should rename and update the import here


Removed this TODO as I don't think it makes sense anymore?

github-actions · 2023-12-08T16:22:59Z

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

github-actions · 2023-12-08T16:28:09Z

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

packages/bpk-component-navigation-bar/src/BpkNavigationBarButtonLink.tsx

packages/bpk-component-navigation-bar/src/BpkNavigationBarIconButton.tsx

packages/bpk-component-navigation-bar/src/BpkNavigationBar.tsx

github-actions · 2023-12-12T11:48:35Z

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

github-actions · 2023-12-12T12:11:00Z

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

github-actions · 2023-12-13T13:11:19Z

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

github-actions · 2023-12-14T10:01:19Z

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

github-actions · 2023-12-14T10:17:03Z

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

anambl

LGTM!

Create on-dark navigation bar

8175414

Iain530 added the minor Non breaking change label Dec 6, 2023

Iain Cattermole added 2 commits December 6, 2023 11:01

Merge branch 'main' of github.com:Skyscanner/backpack into DTO-4949

c3b19b9

Add barStyle to propTypes

ff23ed1

Iain530 changed the title ~~[DTO-4949] Create on-dark variant of BpkNavigationBar~~ [DTO-4949][BpkNavigationBar] Create on-dark variant of BpkNavigationBar Dec 6, 2023

Iain530 marked this pull request as ready for review December 6, 2023 11:49

Merge branch 'main' into DTO-4949

b65d109

Iain Cattermole added 2 commits December 6, 2023 12:30

update snapshots

6bdd683

Merge branch 'DTO-4949' of github.com:Skyscanner/backpack into DTO-4949

3fef381

anambl reviewed Dec 6, 2023

View reviewed changes

packages/bpk-component-navigation-bar/src/themeAttributes.js Outdated Show resolved Hide resolved

anambl reviewed Dec 6, 2023

View reviewed changes

Migrate navbar to typescript

c8f9070

Iain Cattermole added 2 commits December 8, 2023 16:14

Remove themeable property for on-dark

156df51

Remove accidental file renaming

171a177

Iain530 commented Dec 8, 2023

View reviewed changes

packages/bpk-component-navigation-bar/src/BpkNavigationBar-test.tsx Outdated Show resolved Hide resolved

Iain530 commented Dec 8, 2023

View reviewed changes

Iain530 changed the title ~~[DTO-4949][BpkNavigationBar] Create on-dark variant of BpkNavigationBar~~ [DTO-4949][BpkNavigationBar] Create on-dark variant of BpkNavigationBar & Add Typescript Dec 8, 2023

Rebuild css

7ab6a3f

anambl reviewed Dec 11, 2023

View reviewed changes

packages/bpk-component-navigation-bar/src/BpkNavigationBarButtonLink.tsx Show resolved Hide resolved

anambl reviewed Dec 11, 2023

View reviewed changes

packages/bpk-component-navigation-bar/src/BpkNavigationBarIconButton.tsx Show resolved Hide resolved

anambl reviewed Dec 11, 2023

View reviewed changes

packages/bpk-component-navigation-bar/src/BpkNavigationBar.tsx Outdated Show resolved Hide resolved

anambl reviewed Dec 11, 2023

View reviewed changes

packages/bpk-component-navigation-bar/src/BpkNavigationBar.tsx Outdated Show resolved Hide resolved

refactor defaultProps, remove propTypes, add ...rest

a3b0395

keep proptypes for now

11a2e7f

Merge branch 'main' into DTO-4949

e8822b0

Remove ts-expect-error on nav bar import

d965b88

update snapshots

255eebc

Iain530 requested a review from anambl December 14, 2023 10:26

anambl approved these changes Dec 14, 2023

View reviewed changes

anambl merged commit 76e5c39 into main Dec 14, 2023
9 checks passed

anambl deleted the DTO-4949 branch December 14, 2023 14:59

Iain530 mentioned this pull request Dec 15, 2023

[DTO-4949][BpkNavigationBar] Remove prop-types from navbar #3122

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DTO-4949][BpkNavigationBar] Create on-dark variant of BpkNavigationBar & Add Typescript #3110

[DTO-4949][BpkNavigationBar] Create on-dark variant of BpkNavigationBar & Add Typescript #3110

Iain530 commented Dec 6, 2023 •

edited

Iain530 commented Dec 6, 2023

github-actions bot commented Dec 6, 2023

github-actions bot commented Dec 6, 2023 •

edited

github-actions bot commented Dec 6, 2023

Iain530 commented Dec 6, 2023

anambl Dec 6, 2023

Iain530 Dec 6, 2023

Iain530 Dec 8, 2023

anambl Dec 11, 2023

anambl Dec 6, 2023

github-actions bot commented Dec 8, 2023

Iain530 Dec 8, 2023

github-actions bot commented Dec 8, 2023

github-actions bot commented Dec 8, 2023

github-actions bot commented Dec 12, 2023

github-actions bot commented Dec 12, 2023

github-actions bot commented Dec 13, 2023

github-actions bot commented Dec 14, 2023

github-actions bot commented Dec 14, 2023

anambl left a comment

		import PropTypes from 'prop-types';

		// TODO: close button is not really only a close button, we should rename and update the import here

[DTO-4949][BpkNavigationBar] Create on-dark variant of BpkNavigationBar & Add Typescript #3110

[DTO-4949][BpkNavigationBar] Create on-dark variant of BpkNavigationBar & Add Typescript #3110

Conversation

Iain530 commented Dec 6, 2023 • edited

Description

Iain530 commented Dec 6, 2023

github-actions bot commented Dec 6, 2023

github-actions bot commented Dec 6, 2023 • edited

Browser support

github-actions bot commented Dec 6, 2023

Iain530 commented Dec 6, 2023

anambl Dec 6, 2023

Choose a reason for hiding this comment

Iain530 Dec 6, 2023

Choose a reason for hiding this comment

Iain530 Dec 8, 2023

Choose a reason for hiding this comment

anambl Dec 11, 2023

Choose a reason for hiding this comment

anambl Dec 6, 2023

Choose a reason for hiding this comment

github-actions bot commented Dec 8, 2023

Iain530 Dec 8, 2023

Choose a reason for hiding this comment

github-actions bot commented Dec 8, 2023

github-actions bot commented Dec 8, 2023

github-actions bot commented Dec 12, 2023

github-actions bot commented Dec 12, 2023

github-actions bot commented Dec 13, 2023

github-actions bot commented Dec 14, 2023

github-actions bot commented Dec 14, 2023

anambl left a comment

Choose a reason for hiding this comment

Iain530 commented Dec 6, 2023 •

edited

github-actions bot commented Dec 6, 2023 •

edited