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

[core] feat(Card): new "selected" prop #6444

Merged
merged 14 commits into from
Oct 10, 2023
Merged

[core] feat(Card): new "selected" prop #6444

merged 14 commits into from
Oct 10, 2023

Conversation

CPerinet
Copy link
Contributor

@CPerinet CPerinet commented Oct 6, 2023

Screenshot 2023-10-06 at 18 08 46 Screenshot 2023-10-06 at 18 08 49 Screenshot 2023-10-06 at 18 08 58

@adidahiya
Copy link
Contributor

Selected prop

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@CPerinet CPerinet changed the title Cp/card active [core] improvement(Card/SwitchCard): Add selected prop Oct 6, 2023
@CPerinet CPerinet marked this pull request as ready for review October 6, 2023 17:10
@adidahiya
Copy link
Contributor

Selected Control cards

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor

SwitchCard example looks pretty good:

image

I think you need to make some fixes for CardList > SwitchCard, though:

2023-10-06 13 24 25

Also, are there any cases when we'd want to disable this selected styling when a SwitchCard is checked? If so, could we add a prop to disable it, something like showAsSelectedWhenChecked?: boolean with a default value of true? I could imagine this being particularly useful for card lists.

@adidahiya
Copy link
Contributor

Add showAsSelectedWhenChecked prop

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor

Skip cache

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya adidahiya changed the title [core] improvement(Card/SwitchCard): Add selected prop [core] feat(Card): new "selected" prop Oct 10, 2023
@adidahiya
Copy link
Contributor

fix dark theme, add option to ControlCardListExample

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor

fix core:test:typeCheck

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor

This PR is most of the way there, but we need to think through how the new props selected and showAsSelectedWhenChecked will feel with the RadioCard and CheckboxCard components - I don't want to them to feel out of place. Looking into that now...

@adidahiya
Copy link
Contributor

Ok, it seems like these names will work fine with RadioCard and CheckboxCard. Both Radio and Checkbox have the checked prop, although we do not really demo this in the docs-app example components. Setting checked={true} or relying on the uncontrolled input[checked] interaction behavior will work the same for RadioCard & CheckboxCard as it does for SwitchCard in this PR. I think it's good to go. I would like to implement RadioCard & CheckboxCard before the next release so that we can see it all together in the docs.

@adidahiya adidahiya merged commit 33795ce into develop Oct 10, 2023
12 checks passed
@adidahiya adidahiya deleted the cp/card-active branch October 10, 2023 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants