-
Notifications
You must be signed in to change notification settings - Fork 4
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
CED-1754 Create v3 EsFormRadioCards #1525
Conversation
β
Β Linked to Task CED-1754 Β· Create v3 EsFormRadioCards |
@tomleo could you resolve merge conflicts and apply the prism fix? |
π |
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.
Looking pretty good! My comments are mostly geared towards cleaning up the code and setting up downstream repos for success.
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.
Almost there! mainly should just address the focus state of the radio cards
Yes! Done via 1c3d672 |
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! π
π Linked issue
β Type of change
π Description
es-radio-button
andes-radio-button-group
to use the nativeinput
element, and revertedes-ds-styles
back to what they were previously. This fixes styling issues when apply shared styles to the card components.<label><input /></lable>
whereas the button markup is basically<label for="" /><input />
. Implementinges-form-radio-card
andes-form-radio-cards
components also using the nativeinput
element greatly simplifies things.π₯Ό Testing
Manual
π§ Feedback Requested / Focus Areas
v-model
to be set in the wrapping component when also adding radio-cards as slots. I could not figure out how to make this work (aside from a provide/inject implementation that broke other things). It seems like a reasonable trade-off to define a v-model on input fields when you define them, or use v-model on the group component when passing in an options array.π Checklist