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
LG-1719 Add description to radio #1647
Conversation
Size Change: +434 B (0%) Total Size: 829 kB
ℹ️ View Unchanged
|
& + & { | ||
margin-top: 8px; | ||
} | ||
`; | ||
|
||
export const containerSizeStyle: Omit<Record<Size, string>, 'xsmall'> = { | ||
[Size.Small]: css` | ||
grid-template-columns: ${14 + 8}px auto; |
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.
Where are these numbers coming from?
|
||
{description && ( | ||
<Description | ||
className={css` |
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.
Can we move these styles to .styles?
</Label> | ||
|
||
{description && ( | ||
<Description |
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.
Do we need to add an id
to this and pass that to input
with aria-describedby
?
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.
Yep 👍
/** | ||
* Description text rendered under the label. | ||
*/ | ||
description?: string; |
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.
We can also add this to PropTypes.
export const containerStyle = css` | ||
display: grid; | ||
grid-template-areas: 'label label' '. description'; | ||
|
||
& + & { |
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.
You could also remove this bit of code and replace with flex and gap. This way, teams can use flex-direction.
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.
Ah good call!!
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 good - I left a few minor comments.
✍️ Proposed changes
🎟 Jira ticket: LG-1719
✅ Checklist
For bug fixes, new features & breaking changes
yarn changeset
and documented my changesFor new components
🧪 How to test changes