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.

Sign up for GitHub

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

Jump to bottom

feat(ButtonGroup): Add new component ButtonGroup #382

Merged
merged 6 commits into from
Aug 7, 2020
Merged

feat(ButtonGroup): Add new component ButtonGroup #382

merged 6 commits into from
Aug 7, 2020

Conversation

haworku
Copy link
Contributor

@haworku haworku commented Aug 4, 2020

Summary

Adds new component ButtonGroup as per uswds spec

Related Issues or PRs

closes #167

How To Test

Test out the new component in storybook

@haworku haworku added the type: feature New feature or request label Aug 4, 2020
@haworku haworku requested a review from NamibiaTorres August 4, 2020 20:22
ahobson
ahobson approved these changes Aug 5, 2020
View reviewed changes
Copy link
Contributor

@ahobson ahobson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is great. In the USWDS documentation, they mention if using as part of a toolbar, to add role="toolbar". Do you think we should add a prop to control that or are we expecting that to be part of restProps ?

@haworku
Copy link
Contributor Author

haworku commented Aug 6, 2020 •
edited
Loading

@ahobson Yes role=toolbar is included in ...restProps. It is a valid role for a ul but not required - there are several other roles that are also valid. We don't need a prop for that reason - what if they pass the prop and also a role - we'd have to decide which overrides the other.

@trussworks-infra-zz
Copy link

Warnings
⚠️ It looks like there are new component (JSX/TSX) files, but the entrypoint (index.ts) has not changed. - Did you forget to export new components from the library entrypoint?

Generated by 🚫 dangerJS against dd60b88

@ahobson ahobson mentioned this pull request Aug 6, 2020
Closed
@haworku haworku merged commit a7b84d6 into main Aug 7, 2020
@haworku haworku deleted the hw-button-group-167 branch August 7, 2020 11:45
@haworku haworku mentioned this pull request Aug 10, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahobson ahobson ahobson approved these changes

@gidjin gidjin Awaiting requested review from gidjin

@sarboc sarboc Awaiting requested review from sarboc

@suzubara suzubara Awaiting requested review from suzubara

@tinyels tinyels Awaiting requested review from tinyels

@NamibiaTorres NamibiaTorres Awaiting requested review from NamibiaTorres

Assignees
No one assigned
Labels
type: feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

New component: Button group
3 participants
@haworku @trussworks-infra-zz @ahobson