Skip to content
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

feat(styles): add the radio/checkbox card component #1607

Merged
merged 37 commits into from
Aug 17, 2023

Conversation

gfellerph
Copy link
Member

@gfellerph gfellerph commented Jul 5, 2023

Added the radio button and checkbox card component.

@gfellerph gfellerph linked an issue Jul 5, 2023 that may be closed by this pull request
@changeset-bot
Copy link

changeset-bot bot commented Jul 5, 2023

🦋 Changeset detected

Latest commit: 56dd629

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@swisspost/design-system-documentation Minor
@swisspost/design-system-styles Minor
@swisspost/design-system-components-angular Patch
@swisspost/design-system-components Patch
@swisspost/design-system-demo Patch
@swisspost/design-system-documentation-v7 Patch
@swisspost/internet-header Patch
@swisspost/design-system-intranet-header Patch
@swisspost/design-system-components-react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@gfellerph gfellerph mentioned this pull request Jul 5, 2023
2 tasks
@swisspost-bot
Copy link
Contributor

swisspost-bot commented Jul 5, 2023

Preview environment ready: https://preview-1607--swisspost-web-frontend.netlify.app
Preview environment ready: https://preview-1607--swisspost-design-system-next.netlify.app
Preview environment ready: https://preview-1607--swisspost-design-system-next-v7.netlify.app

@gfellerph gfellerph marked this pull request as ready for review July 6, 2023 12:42
@gfellerph gfellerph force-pushed the 1469-add-the-radio-button-card-component branch from 8329317 to 82fff41 Compare August 3, 2023 09:51
gfellerph and others added 6 commits August 4, 2023 08:58
…adiobutton-card.docs.mdx

Co-authored-by: Loïc Fürhoff <12294151+imagoiq@users.noreply.github.com>
…adiobutton-card.docs.mdx

Co-authored-by: Loïc Fürhoff <12294151+imagoiq@users.noreply.github.com>
…heckbox-card.docs.mdx

Co-authored-by: Loïc Fürhoff <12294151+imagoiq@users.noreply.github.com>
…heckbox-card.docs.mdx

Co-authored-by: Loïc Fürhoff <12294151+imagoiq@users.noreply.github.com>
@gfellerph gfellerph requested a review from imagoiq August 4, 2023 07:20
border-color 100ms ease-in-out;

// Checked
&:where(:has(input:checked)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

:has is not supported by Firefox at the moment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a fallback with classes until it is.

…hoice-control.snapshot.stories.ts

Co-authored-by: Loïc Fürhoff <12294151+imagoiq@users.noreply.github.com>
packages/styles/src/components/choice-control-card.scss Outdated Show resolved Hide resolved
packages/styles/src/components/choice-control-card.scss Outdated Show resolved Hide resolved
&:where(:has(input:checked)) {
--post-card-select--hover-bg: #{color.$yellow};
background-color: color.$yellow;
border-color: color.$gray-60;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
border-color: color.$gray-60;
border-color: color.$gray-60;
input {
background-color: color.$white;
}

The radio/checkbox background is not white when check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Adressed

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not working!

Figma:
image
Storybook:
image
image

Comment on lines 68 to 76
&:has(input[aria-invalid]),
&:has(input.is-invalid) {
color: color.$error;
border-color: color.$error;

~ .invalid-feedback {
display: block;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The input and the label should also be red colored in an invalid state.

Copy link
Member Author

Choose a reason for hiding this comment

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

Adressed

Copy link
Contributor

Choose a reason for hiding this comment

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

Not working when checked.
Figma:
image
Storybook:
image
image

@@ -77,6 +77,7 @@ $font-sizes: (
);

$font-size-map: (
12: $font-size-12,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be done in another PR together with a changeset?

Copy link
Contributor

@oliverschuerch oliverschuerch left a comment

Choose a reason for hiding this comment

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

If a user checks/unchecks the checkbox in the preview story, the control for toggling the Checked state is not working anymore.

Copy link
Contributor

@oliverschuerch oliverschuerch left a comment

Choose a reason for hiding this comment

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

Validation states are only reflected with color. There should be at least a hidden text which tells the user, the field is invalid.

Copy link
Contributor

@oliverschuerch oliverschuerch left a comment

Choose a reason for hiding this comment

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

HCM Styles are not well.
Use smaller borders for cards and higlight color for selected cards.

gfellerph and others added 4 commits August 11, 2023 15:23
…hoice-control.ts

Co-authored-by: Oliver Schürch <oliver.schuerch@post.ch>
…hoice-control.ts

Co-authored-by: Oliver Schürch <oliver.schuerch@post.ch>
…adiobutton-card.docs.mdx

Co-authored-by: Oliver Schürch <oliver.schuerch@post.ch>
@gfellerph
Copy link
Member Author

gfellerph commented Aug 11, 2023

HCM Styles are not well. Use smaller borders for cards and higlight color for selected cards.

Updated the hover styles, but I would not use the selectedItem color as background, I think it's a bit disruptive to the otherwise very simple HCM.

Also, a 10px border is coming from storybook, which has some broken HCM styles anyway.

@sonarcloud
Copy link

sonarcloud bot commented Aug 11, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@gfellerph gfellerph added the 📦 migrations Related to the @swisspost/design-system-migrations package label Aug 17, 2023
@gfellerph gfellerph merged commit c1dee3b into main Aug 17, 2023
11 checks passed
@gfellerph gfellerph deleted the 1469-add-the-radio-button-card-component branch August 17, 2023 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 migrations Related to the @swisspost/design-system-migrations package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add the radio button card component
4 participants