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

USWDS - Button: define default type in JSON #5247

Merged
merged 1 commit into from
May 8, 2023
Merged

Conversation

mahoneycm
Copy link
Contributor

@mahoneycm mahoneycm commented Apr 20, 2023

Summary

Specifies type in usa-button.json to fix html component code

Breaking change

This is not a breaking change.

Change log PR

uswds/uswds-site#2093

Related issue

Closes #5246

Preview link

Button: storybook →
Demo USWDS site with correct button type showing →

Problem statement

Our html component code for usa-button would display an empty type attribute for usa-button

<button type="" class="usa-button usa-button--base">Default</button>

Expected:

<button type="button" class="usa-button usa-button--base">Default</button>

Solution

Add the default type attribute to the usa-button json file so that webpack correctly fills the variable in the twig file.

Before, this value was being set by storybook controls, which do not seem to make their way to the html templates once they're built.

Testing and review

  1. Visit the button preview and make sure storybook type controls are working as expected
  2. Visit html-templates/usa-button.html and check for correct type attribute
  3. Check other button variants for the same

View change on site

  1. Visit test site repo button component page
  2. Check component code and make sure the correct button type is showing

image

Note

This seems to be the most direct way to populate this field, but it does override the defaultValue field within the storybook controls.

If you change defaultValue within usa-button.stories.js, the default button story will always default to type="button" while the other variants will change to the set value. This can still be changed using the radio buttons though!

Currently, the defaultValue is button so the desired preview is still maintained. I just wanted to flag it in case!

Before opening this PR, make sure you’ve done whichever of these applies to you:

  • Confirm that this code follows the 18F Front End Coding Style Guide and Accessibility Guide.
  • Run git pull origin [base branch] to pull in the most recent updates from your base and check for merge conflicts. (Often, the base branch is develop).
  • Run npm test and confirm that all tests pass.

Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this! Alternatively we can use the | default() filter in twig.

Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

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

This looks good! Confirmed that type now appears in usa-button.html and that there are no empty button types in html-templates.

@mejiaj mejiaj requested a review from thisisdano April 21, 2023 20:33
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.

Buttons are rendering with empty "type"
4 participants