Skip to content

[PANGOO-2905][BpkCheckbox]Checkbox With SVG #3774

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 17 commits into from
Mar 27, 2025
Merged

[PANGOO-2905][BpkCheckbox]Checkbox With SVG #3774

merged 17 commits into from
Mar 27, 2025

Conversation

senzg
Copy link
Contributor

@senzg senzg commented Mar 21, 2025 •
edited
Loading

Before After
image image

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
  • Accessibility tests
    • The following checks were performed:
      • Ability to navigate using a keyboard only
      • Zoom functionality (Deque University explanation):
        • The page SHOULD be functional AND readable when only the text is magnified to 200% of its initial size
        • Pages must reflow as zoom increases up to 400% so that content continues to be presented in only one column i.e. Content MUST NOT require scrolling in two directions (both vertically and horizontally)
      • Ability to navigate using a screen reader only
  • 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

@senzg senzg added the patch Patch production bug label Mar 21, 2025
@skyscanner-backpack-bot
Copy link

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

@skyscanner-backpack-bot
Copy link

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

@skyscanner-backpack-bot
Copy link

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

@skyscanner-backpack-bot
Copy link

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

@senzg senzg changed the title [WIP]PANGOO-2905/checkbox-with-svg [PANGOO-2905][BpkCheckbox]Checkbox With SVG Mar 26, 2025
@skyscanner-backpack-bot
Copy link

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

@senzg
Copy link
Contributor Author

senzg commented Mar 26, 2025

Percy failed as expected, because the checkmark changed.

@skyscanner-backpack-bot
Copy link

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

GCpigsic
GCpigsic reviewed Mar 26, 2025
View reviewed changes
packages/bpk-mixins/_forms.scss Outdated
Comment on lines 637 to 639
background-repeat: no-repeat;
background-position: $bpk-one-pixel-rem * 0.5 center;
background-size: calc(100% - $bpk-one-pixel-rem * 2.5) auto;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need to declare background-repeat background-position background-size again? seems we already declare them in L631-L633

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes, thanks for your reminding! They are deleted and tested

packages/bpk-mixins/_forms.scss Outdated
Comment on lines 632 to 633
background-position: $bpk-one-pixel-rem * 0.5 center;
background-size: calc(100% - $bpk-one-pixel-rem * 2.5) auto;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just out of curiosity, how do we get 0.5 and 2.5 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's only a subjective number to make the visual effect look good. The problem is that the left padding and the right padding of the svg I get are slightly different, it is not good looking without this adjustment.

@skyscanner-backpack-bot
Copy link

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

@skyscanner-backpack-bot
Copy link

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

@skyscanner-backpack-bot
Copy link

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

GCpigsic
GCpigsic reviewed Mar 27, 2025
View reviewed changes
packages/bpk-mixins/_forms.scss
background-image: url('data:image/svg+xml,%3Csvg%20width%3D%2213%22%20height%3D%229%22%20viewBox%3D%220%200%2013%209%22%20fill%3D%22none%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%3E%3Cpath%20d%3D%22M2.35352%203.64648L5.5%207.5L11.5%201.5%22%20stroke%3D%22white%22%20stroke-width%3D%223%22%20stroke-linecap%3D%22round%22%20stroke-linejoin%3D%22round%22%2F%3E%3C%2Fsvg%3E');
background-repeat: no-repeat;
background-position: $bpk-one-pixel-rem center;
background-size: calc(100% - $bpk-one-pixel-rem * 2.5) auto;
Copy link
Contributor

Choose a reason for hiding this comment

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

note here: the design has confirmed with the backpack designer by @senzg

@senzg senzg merged commit bec33e3 into main Mar 27, 2025
9 checks passed
@senzg senzg deleted the PANGOO-2905/checkbox branch March 27, 2025 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GCpigsic GCpigsic GCpigsic approved these changes

Assignees
No one assigned
Labels
patch Patch production bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@senzg @GCpigsic @olliecurtis