-
Notifications
You must be signed in to change notification settings - Fork 601
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: add vertical and horizontal orientation for radio-group #3082
Conversation
b245d99
to
1bd354e
Compare
packages/web-components/fast-components/src/radio-group/radio-group.template.ts
Outdated
Show resolved
Hide resolved
packages/web-components/fast-components/src/radio-group/radio-group.ts
Outdated
Show resolved
Hide resolved
packages/web-components/fast-components/src/radio-group/fixtures/base.html
Outdated
Show resolved
Hide resolved
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.
Agree with all Jane's comments. If we get those addressed, we should be good to go on this.
packages/web-components/fast-components/src/radio-group/radio-group.ts
Outdated
Show resolved
Hide resolved
packages/web-components/fast-components/src/radio-group/radio-group.styles.ts
Show resolved
Hide resolved
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.
I love seeing the simplicity here. I’m going to hold on approving this only because I think it’s important to get perspective from @nicholasrice as it was a conversation with him on discord which led to the proposal tracking this. As long as we reach consensus to implement this, I’m good with this. I do want to be sure we give time for his feedback here.
b720d03
to
3ca42e5
Compare
3ca42e5
to
df66712
Compare
Description
In response to #3069 adding the orientation attribute to fast-radio-group to allow for vertical orientation and a default of horizontal.
Motivation & context
Motivation described in proposal #3069.
Issue type checklist
Is this a breaking change?
Process & policy checklist