-
Notifications
You must be signed in to change notification settings - Fork 202
[IRN-6177] [BpkCardList] Fix issue with carousel alignment #4004
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes carousel alignment issues in BpkCardList when using row-to-rail mode. The changes address problems with drop shadows being cut off, incorrect card widths, and container edge positioning.
- Simplified the flex-basis calculation for mobile breakpoints
- Added specific styling for row mode with negative margins and proper flex-basis calculations
- Maintained existing rail mode behavior without the alignment fixes
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
packages/bpk-component-card-list/src/BpkCardListRowRail/BpkCardListCarousel.module.scss
Show resolved
Hide resolved
packages/bpk-component-card-list/src/BpkCardListRowRail/BpkCardListCarousel.module.scss
Outdated
Show resolved
Hide resolved
|
Visit https://backpack.github.io/storybook-prs/4004 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/4004 to see this build running in a browser. |
Hey Jimmy cook (@jimmycook) Can you explain this more? I think we can also fix this on mobile and I tested locally it works well, I also raised a commit to change that. Storybook is here https://backpack.github.io/storybook-prs/4004/?path=/story/bpk-component-card-list--row-to-rail Can you have a look if it works? If it works, we can approve and merge it. Otherwise, I'll revert that commit. |
|
Visit https://backpack.github.io/storybook-prs/4004 to see this build running in a browser. |
Good suggestion but I think we have a problem |
|
Visit https://backpack.github.io/storybook-prs/4004 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/4004 to see this build running in a browser. |
* [CLOV-964] Export legacy props and types * lint
* BpkCheckbox is migrated to Typescirpt now and export type BpkCheckboxProps
* [CLOV-950] [BpkButton] Migrate BpkButton to BpkButtonV2 * update type order * update props and other path
Bumps [glob](https://github.com/isaacs/node-glob) to 10.5.0 and updates ancestor dependency . These dependencies need to be updated together. Updates `glob` from 10.4.5 to 10.5.0 - [Changelog](https://github.com/isaacs/node-glob/blob/main/changelog.md) - [Commits](isaacs/node-glob@v10.4.5...v10.5.0) Updates `glob` from 11.0.3 to 11.1.0 - [Changelog](https://github.com/isaacs/node-glob/blob/main/changelog.md) - [Commits](isaacs/node-glob@v10.4.5...v10.5.0) --- updated-dependencies: - dependency-name: glob dependency-version: 10.5.0 dependency-type: indirect - dependency-name: glob dependency-version: 11.1.0 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: kerrie-wu <52308828+kerrie-wu@users.noreply.github.com>
Bumps [core-js](https://github.com/zloirock/core-js/tree/HEAD/packages/core-js) from 3.45.1 to 3.46.0. - [Release notes](https://github.com/zloirock/core-js/releases) - [Changelog](https://github.com/zloirock/core-js/blob/master/CHANGELOG.md) - [Commits](https://github.com/zloirock/core-js/commits/v3.46.0/packages/core-js) --- updated-dependencies: - dependency-name: core-js dependency-version: 3.46.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: kerrie-wu <52308828+kerrie-wu@users.noreply.github.com>
* covert BpkFielset to TS * add check and label type check * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * change any type * fix typecheck error --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ser Agent (#4052) * Override padding alignment set by Safari User Agent * simplify
… _ or long words in general (#4054)
* fix up and down blur issue * fix up down issue * fix fistHightLight * suppress the runtime error * modify comment * prevent autosuggest flickering on input change * optimise highlight Index function * fix typecheck error * fix test onSuggestionSelected
Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 3.14.1 to 3.14.2. - [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md) - [Commits](nodeca/js-yaml@3.14.1...3.14.2) --- updated-dependencies: - dependency-name: js-yaml dependency-version: 3.14.2 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: kerrie-wu <52308828+kerrie-wu@users.noreply.github.com>
Bumps the babel group with 2 updates in the / directory: [@babel/preset-react](https://github.com/babel/babel/tree/HEAD/packages/babel-preset-react) and [@babel/preset-typescript](https://github.com/babel/babel/tree/HEAD/packages/babel-preset-typescript). Updates `@babel/preset-react` from 7.27.1 to 7.28.5 - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.28.5/packages/babel-preset-react) Updates `@babel/preset-typescript` from 7.27.0 to 7.28.5 - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.28.5/packages/babel-preset-typescript) --- updated-dependencies: - dependency-name: "@babel/preset-react" dependency-version: 7.28.5 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: babel - dependency-name: "@babel/preset-typescript" dependency-version: 7.28.5 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: babel ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: kerrie-wu <52308828+kerrie-wu@users.noreply.github.com>
…4060) * Bump @skyscanner/stylelint-config-skyscanner from 13.1.0 to 14.2.0 Bumps [@skyscanner/stylelint-config-skyscanner](https://github.com/Skyscanner/stylelint-config-skyscanner) from 13.1.0 to 14.2.0. - [Release notes](https://github.com/Skyscanner/stylelint-config-skyscanner/releases) - [Changelog](https://github.com/Skyscanner/stylelint-config-skyscanner/blob/main/CHANGELOG.md) - [Commits](Skyscanner/stylelint-config-skyscanner@13.1.0...14.2.0) --- updated-dependencies: - dependency-name: "@skyscanner/stylelint-config-skyscanner" dependency-version: 14.2.0 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * fix check error --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Kerrie Wu <kerrie.wu@skyscanner.net>
* Bump the storybook group across 1 directory with 5 updates Bumps the storybook group with 5 updates in the / directory: | Package | From | To | | --- | --- | --- | | [@storybook/addon-a11y](https://github.com/storybookjs/storybook/tree/HEAD/code/addons/a11y) | `9.1.13` | `10.1.2` | | [@storybook/addon-docs](https://github.com/storybookjs/storybook/tree/HEAD/code/addons/docs) | `9.1.13` | `10.1.2` | | [@storybook/addon-webpack5-compiler-babel](https://github.com/storybookjs/addon-webpack5-compiler-babel) | `3.0.6` | `4.0.0` | | [@storybook/react-webpack5](https://github.com/storybookjs/storybook/tree/HEAD/code/frameworks/react-webpack5) | `9.1.13` | `10.1.2` | | [storybook](https://github.com/storybookjs/storybook/tree/HEAD/code/core) | `9.1.13` | `10.1.2` | Updates `@storybook/addon-a11y` from 9.1.13 to 10.1.2 - [Release notes](https://github.com/storybookjs/storybook/releases) - [Changelog](https://github.com/storybookjs/storybook/blob/next/CHANGELOG.md) - [Commits](https://github.com/storybookjs/storybook/commits/v10.1.2/code/addons/a11y) Updates `@storybook/addon-docs` from 9.1.13 to 10.1.2 - [Release notes](https://github.com/storybookjs/storybook/releases) - [Changelog](https://github.com/storybookjs/storybook/blob/next/CHANGELOG.md) - [Commits](https://github.com/storybookjs/storybook/commits/v10.1.2/code/addons/docs) Updates `@storybook/addon-webpack5-compiler-babel` from 3.0.6 to 4.0.0 - [Release notes](https://github.com/storybookjs/addon-webpack5-compiler-babel/releases) - [Changelog](https://github.com/storybookjs/addon-webpack5-compiler-babel/blob/main/CHANGELOG.md) - [Commits](storybookjs/addon-webpack5-compiler-babel@v3.0.6...v4.0.0) Updates `@storybook/react-webpack5` from 9.1.13 to 10.1.2 - [Release notes](https://github.com/storybookjs/storybook/releases) - [Changelog](https://github.com/storybookjs/storybook/blob/next/CHANGELOG.md) - [Commits](https://github.com/storybookjs/storybook/commits/v10.1.2/code/frameworks/react-webpack5) Updates `storybook` from 9.1.13 to 10.1.2 - [Release notes](https://github.com/storybookjs/storybook/releases) - [Changelog](https://github.com/storybookjs/storybook/blob/next/CHANGELOG.md) - [Commits](https://github.com/storybookjs/storybook/commits/v10.1.2/code/core) --- updated-dependencies: - dependency-name: "@storybook/addon-a11y" dependency-version: 10.1.2 dependency-type: direct:development update-type: version-update:semver-major dependency-group: storybook - dependency-name: "@storybook/addon-docs" dependency-version: 10.1.2 dependency-type: direct:development update-type: version-update:semver-major dependency-group: storybook - dependency-name: "@storybook/addon-webpack5-compiler-babel" dependency-version: 4.0.0 dependency-type: direct:development update-type: version-update:semver-major dependency-group: storybook - dependency-name: "@storybook/react-webpack5" dependency-version: 10.1.2 dependency-type: direct:development update-type: version-update:semver-major dependency-group: storybook - dependency-name: storybook dependency-version: 10.1.2 dependency-type: direct:development update-type: version-update:semver-major dependency-group: storybook ... Signed-off-by: dependabot[bot] <support@github.com> * fix storybook update issue * fix conflict and issue --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: kerrie-wu <52308828+kerrie-wu@users.noreply.github.com> Co-authored-by: Kerrie Wu <kerrie.wu@skyscanner.net>
* Bump jws from 3.2.2 to 3.2.3 Bumps [jws](https://github.com/brianloveswords/node-jws) from 3.2.2 to 3.2.3. - [Release notes](https://github.com/brianloveswords/node-jws/releases) - [Changelog](https://github.com/auth0/node-jws/blob/master/CHANGELOG.md) - [Commits](auth0/node-jws@v3.2.2...v3.2.3) --- updated-dependencies: - dependency-name: jws dependency-version: 3.2.3 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * update package-lock.json --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: gc.zhu <gc.zhu@skyscanner.net>
…#4076) * [CLOV-907][BpkButton] Update BpkButton link and linkOnDark type style * add implicit to BpkPaginationNudger and BpkFloatingNotification and ExpandAccessoryContent * adjust linkOnDark hover color * add Links example * update link on dark hover style * remove full props in README * update BpkPaginationNudger to iconOnly * remove bpk-buttob-link-disabled * button link aligned * support inline display svg * add button-link-type README * rename bpk-button--link-icon-only-aligned * rename bpk-button--link-icon-only
…to BpkButtonV2 (#4078) * remove BpkButton code and alias BpkButton to BpkButtonV2 * refine themeAttributes file structure * update readme * correct file format * udpate readme * update readme --------- Co-authored-by: Ezreal Yang <supremeyh@126.com>
Bumps [lint-staged](https://github.com/lint-staged/lint-staged) from 16.1.5 to 16.2.7. - [Release notes](https://github.com/lint-staged/lint-staged/releases) - [Changelog](https://github.com/lint-staged/lint-staged/blob/main/CHANGELOG.md) - [Commits](lint-staged/lint-staged@v16.1.5...v16.2.7) --- updated-dependencies: - dependency-name: lint-staged dependency-version: 16.2.7 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: IrinaWei <irina.wei@skyscanner.net>
Bumps [glob](https://github.com/isaacs/node-glob) from 11.1.0 to 13.0.0. - [Changelog](https://github.com/isaacs/node-glob/blob/main/changelog.md) - [Commits](isaacs/node-glob@v11.1.0...v13.0.0) --- updated-dependencies: - dependency-name: glob dependency-version: 13.0.0 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: IrinaWei <irina.wei@skyscanner.net>
* Initial plan * Initial exploration Co-authored-by: gert-janvercauteren <728889+gert-janvercauteren@users.noreply.github.com> * Migrate BpkLink component to TypeScript Co-authored-by: gert-janvercauteren <728889+gert-janvercauteren@users.noreply.github.com> * Add missing semicolon in examples.tsx Co-authored-by: gert-janvercauteren <728889+gert-janvercauteren@users.noreply.github.com> * Remove unused @ts-expect-error directives for bpk-component-link imports Co-authored-by: gert-janvercauteren <728889+gert-janvercauteren@users.noreply.github.com> * Remove duplicate JS files Co-authored-by: gert-janvercauteren <728889+gert-janvercauteren@users.noreply.github.com> * Update packages/bpk-component-link/src/BpkLink.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update examples/bpk-component-link/examples.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update packages/bpk-component-link/src/BpkLink.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix TypeScript errors: convert null to undefined for href and rel attributes Co-authored-by: gert-janvercauteren <728889+gert-janvercauteren@users.noreply.github.com> * Make href prop required in BpkLink to match original Flow type and ensure accessibility Co-authored-by: gert-janvercauteren <728889+gert-janvercauteren@users.noreply.github.com> * Extend AnchorHTMLAttributes instead of using index signature for better Storybook docgen support Co-authored-by: gert-janvercauteren <728889+gert-janvercauteren@users.noreply.github.com> * Extend ButtonHTMLAttributes for BpkButtonLink to match BpkLink pattern and improve Storybook docgen Co-authored-by: gert-janvercauteren <728889+gert-janvercauteren@users.noreply.github.com> * omit props * address rel --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: gert-janvercauteren <728889+gert-janvercauteren@users.noreply.github.com> Co-authored-by: Gert-Jan Vercauteren <gert-jan.vercauteren@skyscanner.net> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Ezreal Yang <supremeyh@126.com>
Co-authored-by: Flora Cheng <flora.cheng@skyscanner.net>
* feat: added variant size medium to bpkPrice * fix: fixed spacing issues picked up in PR * chore: run tests --------- Co-authored-by: Gert-Jan Vercauteren <gert-jan.vercauteren@skyscanner.net>
* [CLOV-657] Add as prop to support BpkLink * update ref type * use BpkPanel in storybook examples * remove BpkButtonLink in README and storybook * simplify with react type * introduce const LINK_AS * update LINK_AS cases * update as Element * update snapshots * ArgTypes specify API * add href null * common-types * update import and export path * remove LINK_AS * remove BpkLinkPolymorphicProps * update processAnchorProps and getClassNames * remove LinksAs type
* [CLOV-909][BpkButtonLink] Mark BpkButtonLink deprecated * type label and pass rest
…ase (#4097) * [CLOV-957][BpkButton] Replace BpkButtonV2 with BpkButton across codebase * update figma path
* export default them * Update packages/bpk-component-autosuggest/index.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Bump storybook from 10.1.2 to 10.1.11 Bumps [storybook](https://github.com/storybookjs/storybook/tree/HEAD/code/core) from 10.1.2 to 10.1.11. - [Release notes](https://github.com/storybookjs/storybook/releases) - [Changelog](https://github.com/storybookjs/storybook/blob/next/CHANGELOG.md) - [Commits](https://github.com/storybookjs/storybook/commits/v10.1.11/code/core) --- updated-dependencies: - dependency-name: storybook dependency-version: 10.1.11 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> * Update package-lock.json --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Flora Cheng <flora.cheng@skyscanner.net>
* Initial plan * Migrate BpkCode component to TypeScript Co-authored-by: Faye-Xiao <20058385+Faye-Xiao@users.noreply.github.com> * Refactor BpkCode to use consistent className handling approach Co-authored-by: Faye-Xiao <20058385+Faye-Xiao@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Replace snapshot tests with Jest expect assertions Co-authored-by: Faye-Xiao <20058385+Faye-Xiao@users.noreply.github.com> * Remove unused @ts-expect-error directive for BpkCode import Co-authored-by: Faye-Xiao <20058385+Faye-Xiao@users.noreply.github.com> * Update README to tsx and add rest props type support Co-authored-by: Faye-Xiao <20058385+Faye-Xiao@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Faye-Xiao <20058385+Faye-Xiao@users.noreply.github.com> Co-authored-by: Faye <faye.xiao@skyscanner.net> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This reverts commit eaf2cbe.
|
Visit https://backpack.github.io/storybook-prs/4004 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/4004 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/4004 to see this build running in a browser. |
Faye (Faye-Xiao)
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.

This change is to address the issues with alignment when using row to rail.
There are a few concerns:
This change fixes is by when we are in the row state, we can have negative margin equal to the padding on the cards, and adjust the flex basis of the cards to fill the space correctly.
On the advice of Adam Wilson, we've changed mobile to be edge to edge now, with padding on the sides to ensure it aligns with existing content, we're using the base sizing for this.

Remember to include the following changes:
[Clover-123][BpkButton] Updating the colourREADME.md(If you have created a new component)README.md