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.

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: New Component ProcessList MVP #1107

Merged
merged 23 commits into from
Apr 27, 2021

Conversation

SirenaBorracha
Copy link
Contributor

@SirenaBorracha SirenaBorracha commented Apr 9, 2021

Summary

This PR adds the USWDS ProcessList component in an MVP state.

Related Issues or PRs

#979

An enhancement that we may want to add as part of this GitHub issue would be to create a ProcessListItemHeader subcomponent. This would allow us to offer an out of the box way for our users to pass a heading that would automatically be styled by the usa-process-list__heading class irrespective of what element type the user passed.

This enhancement would meet the USWDS Accessibility guideline, "Use semantic heading levels," that states, "Though our default code uses an <h4>, use the correct heading level with the class name usa-process-list__heading in your own implementation."

How To Test

Check out this branch and run yarn test to execute tests and yarn storybook to see the new component in action.

Alternatively, you can scroll to the bottom of this page and click "Show environments" on the right above the comment input box to access this component in Storybook.

Screenshots (optional)

Default

Screen Shot 2021-04-08 at 5 46 36 PM

No text, custom sizing

Screen Shot 2021-04-08 at 5 48 18 PM

Custom Sizing

Screen Shot 2021-04-08 at 5 48 45 PM

@SirenaBorracha
Copy link
Contributor Author

@haworku and @brandonlenz, @suzubara requested that I get your opinions on whether or not we care to provide a ProcessListItemHeader subcomponent. As is, ProcessList is flexible but does not provide all of the tools that our users might benefit from.

Do either of you feel strongly in favor of adding the header subcomponent?

@brandonlenz brandonlenz added the status: hold merge This indicates not to merge the PR as there is a development dependency/other reason to hold off. label Apr 9, 2021
@brandonlenz brandonlenz added this to the USWDS 2.10.0 milestone Apr 9, 2021
@SirenaBorracha SirenaBorracha linked an issue Apr 9, 2021 that may be closed by this pull request
@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook April 10, 2021 00:10 Inactive
Copy link
Contributor

@brandonlenz brandonlenz left a comment

Choose a reason for hiding this comment

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

@haworku and @brandonlenz, @suzubara requested that I get your opinions on whether or not we care to provide a ProcessListItemHeader subcomponent. As is, ProcessList is flexible but does not provide all of the tools that our users might benefit from.

Do either of you feel strongly in favor of adding the header subcomponent?

Personally, I'm for creating the subcomponent. Since it should be able to be any type of heading (or p) tag, we could expose a variant prop or similar to allow the implementer to select what type of markup should be used.

}: ProcessListProps & JSX.IntrinsicElements['ol']): React.ReactElement => {
const classes = classnames('usa-process-list', className)
return (
<>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this Fragment is needed

Copy link
Contributor

@brandonlenz brandonlenz left a comment

Choose a reason for hiding this comment

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

One small thing I noticed. This looks really good!

I added a separate comment about the header.

@brandonlenz
Copy link
Contributor

brandonlenz commented Apr 12, 2021

@haworku and @brandonlenz, @suzubara requested that I get your opinions on whether or not we care to provide a ProcessListItemHeader subcomponent. As is, ProcessList is flexible but does not provide all of the tools that our users might benefit from.
Do either of you feel strongly in favor of adding the header subcomponent?

Personally, I'm for creating the subcomponent. Since it should be able to be any type of heading (or p) tag, we could expose a variant prop or similar to allow the implementer to select what type of markup should be used.

Maybe a generic type prop so the interface might looks something like:

<ProcessListHeading<'h1'>>"Heading text"</ProcessListHeading>. 
Disclaimer ⚠️ I have not tried that, just thought it might be nicer than having a headerType prop if it works

edit: eh, yea this may not be a good use for a generic param since you constrain the values of the generic via extends rather than explicit type definition.

@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook April 12, 2021 18:13 Inactive
@SirenaBorracha
Copy link
Contributor Author

Created issue #1134 for the ProcessListItemHeader subcomponent

@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook April 13, 2021 20:01 Inactive
@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook April 20, 2021 17:15 Inactive
@brandonlenz brandonlenz removed the status: hold merge This indicates not to merge the PR as there is a development dependency/other reason to hold off. label Apr 20, 2021
@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook April 20, 2021 19:41 Inactive
@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook April 23, 2021 00:15 Inactive
suzubara
suzubara previously approved these changes Apr 23, 2021
yarn.lock Outdated
uswds@2.10.3:
version "2.10.3"
resolved "https://registry.yarnpkg.com/uswds/-/uswds-2.10.3.tgz#16d34cee81897762d6d69d3ac83aa55129826fa6"
integrity sha512-krNRzx1jRzOJpuH/qtmQhd5zxnXTaDVqrPNYT99sJbxzWUqjb1zZHh3jFNo+xKDpNuiO0XMPwZwlaSp2YdZ3Ag==

Copy link
Contributor

Choose a reason for hiding this comment

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

this file shouldn't have any changes

yarn.lock Outdated Show resolved Hide resolved
@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook April 26, 2021 23:05 Inactive
@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook April 27, 2021 18:43 Inactive
@brandonlenz brandonlenz self-requested a review April 27, 2021 20:33
Copy link
Contributor

@brandonlenz brandonlenz left a comment

Choose a reason for hiding this comment

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

Awesome job!

@SirenaBorracha SirenaBorracha merged commit 1bc0f93 into main Apr 27, 2021
@SirenaBorracha SirenaBorracha deleted the ak-new-component-process-list-979 branch April 27, 2021 20:57
SirenaBorracha added a commit that referenced this pull request May 5, 2021
## [1.17.0](1.16.0...1.17.0) (2021-05-05)


### Features

* Checkbox Tile Variant ([#1104](#1104)) ([9936c4a](9936c4a))
* Implement ProcessListHeading subcomponent ([#1162](#1162)) ([964e34c](964e34c))
* New Component ProcessList MVP ([#1107](#1107)) ([1bc0f93](1bc0f93))
* New Component SiteAlert ([#1099](#1099)) ([c1e88e0](c1e88e0))
* New Component SummaryBox ([#1098](#1098)) ([b2279b4](b2279b4))
* Radio Button Tile Variant ([#1101](#1101)) ([a2f40a0](a2f40a0))
* Update Grid components to render any type of element ([#1166](#1166)) ([07468c8](07468c8)), closes [#1194](#1194)
* Update Search component to support i18n ([#1192](#1192)) ([5241d15](5241d15))
* Update Table to 2.10.0 implementation  ([#1110](#1110)) ([117a6c7](117a6c7))
brandonlenz pushed a commit that referenced this pull request May 12, 2021
brandonlenz pushed a commit that referenced this pull request May 12, 2021
## [1.17.0](1.16.0...1.17.0) (2021-05-05)


### Features

* Checkbox Tile Variant ([#1104](#1104)) ([9936c4a](9936c4a))
* Implement ProcessListHeading subcomponent ([#1162](#1162)) ([964e34c](964e34c))
* New Component ProcessList MVP ([#1107](#1107)) ([1bc0f93](1bc0f93))
* New Component SiteAlert ([#1099](#1099)) ([c1e88e0](c1e88e0))
* New Component SummaryBox ([#1098](#1098)) ([b2279b4](b2279b4))
* Radio Button Tile Variant ([#1101](#1101)) ([a2f40a0](a2f40a0))
* Update Grid components to render any type of element ([#1166](#1166)) ([07468c8](07468c8)), closes [#1194](#1194)
* Update Search component to support i18n ([#1192](#1192)) ([5241d15](5241d15))
* Update Table to 2.10.0 implementation  ([#1110](#1110)) ([117a6c7](117a6c7))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New Component: Process list
4 participants