-
Notifications
You must be signed in to change notification settings - Fork 2
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
[IOPLT-237] Refinement of ListItemRadio
and RadioGroup
#136
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.
Just a couple of little things:
- The first two
id
fields provided in the example are the same, so if you try to select the second radio item, you will have two items selected at the same time. - The
ListItemRadio
item should be disabled during theloading
state. Currently the circle has the same color as the enabled item.
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.
As for the skeleton component, right now we're only managing one scenario: when we're sure that the title and description are present. But if we look at the different flows (for example, the "Choose your payment method" screen), there are other possible combinations:
- Title only
- Title and icon (without description)
- The full one: icon, title and description
We should provide the flexibility to let the developer choose the right one depending on the context. Feel free to get in touch with me if you would like to see the different scenarios from our design projects.
addressed in d0e83ce |
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.
A specific skeleton case was missing (base with icon), so I refactored the loadingProps
with boolean values. In short:
- a single line as base behavior, when the
state
is set totrue
- a single line with an icon, if
skeletonIcon
is set totrue
- a single line with an icon and description, if both
skeletonIcon
andskeletonDescription
are set totrue
With the most complex skeleton, we have:
loadingProps: {
state: true,
skeletonDescription: true,
skeletonIcon: true
}
Also used the optional chaining operator to replace loadingProps && loadingProps.state
with loadingProps?.state
Let me know if it makes sense.
ListItemRadio
and RadioGroupListItemRadio
and RadioGroup
Short description
This PR introduces a refinement to
ListItemRadio
component to handle loading state and support payment logo rendering in place of an icon in exclusion of it.ListItemRadio props
startImage
replaces the flaticon
orpaymentLogo
props to solve an issue inRadioGroup
usage that was not recognizing the correct prop spec in its usage.List of changes proposed in this pull request
ListItemRadio
RadioGroup
How to test
Check the
Selection
page on example app.