USWDS - Button: Add basic support for usa-icon in buttons #5398

mejiaj · 2023-07-20T15:59:14Z

Summary

Improved the vertical alignment of usa-icon elements inside of usa-button. Use the new $theme-button-icon-gap setting to set the width of the horizontal gap between the button's text and icon.

New features:

Improved icon alignment in buttons. Buttons that include an inline usa-icon are now visually centered.
Improved control of inner content. Buttons use flexbox for vertical alignment and column spacing. Added a new setting $theme-button-icon-gap: 1 !default; that takes units for inner content.
New Button controls in Storybook. There are two new controls for internal testing: add_icon & icon_name.

Control: add_icon

A toggle for including an icon in stories.

Control: icon_name (Depends on add_icon)

A dropdown of all USA icons with add_circle_outline as the default. Icon was mentioned in the original issue and was used as the basis for styling.

Breaking change

This is not a breaking change.

Related issue

Closes #5359.

Related pull requests

Changelog entry
Related to USWDS - Button Group: Improve appearance of buttons with wrapping text #5657. Fixes alignment issue mentioned in PR.

Preview links

Button with icon.
Button accent cool variant. Shows new controls available to button component.
Develop branch preview

Problem statement

USA Icons in buttons weren't centered vertically.

Before

After

Added icon to both sides to illustrate spacing via $theme-button-icon-gap.

Solution

Improved support for buttons with inline usa-icons using inline-flex.

Why this approach was chosen

Improved control over inner spacing via the column-gap property.
Icons before/after will be properly spaced.
The move from inline-block → inline-flex doesn't seem like a big departure in display.

~~Follows a similar pattern to what's been done in breadcrumb:14 & external link:32-40. A small enhancement seemed less heavy handed than other solution.~~

How I implemented

Switched from inline-block → inline-flex. Added properties for column gap and justify content (for centering text).

~~Added vertical alignment and negative margin to visually center.~~

Alternatives

Using the :has() selector to use inline-grid ~~or inline-flex~~. These features provide more control, but have their own problems:

:has() doesn't have crossbrowser support.
There are conflicts with postcsso and :has() pseudoselector. The plugin for postcss requires a JS polyfill to be included & has performance issues.

Major changes

Variable	Default	Type	Description
`$theme-button-icon-gap`	1	units	The inner flexbox column gap size.

Testing and review

Icon alignment

Visit button icon preview link.
Confirm icon is visually centered.

Storybook Controls

Go to any button variant, like Accent warm.
Confirm new control add_icon is visible.
Toggle to True.
Confirm that new control icon_name shows as a dropdown for setting icon.
Confirm that all USA icons are available as options in dropdown.
Confirm that icon changes on selection.

Testing checklist

Icons in buttons should be visually centered.
Button stories have a control to toggle icons.
- If Include icon is true, show a dropdown of icons.
- Icons should update on select.
There should be no html:build errors.

mahoneycm

Diggin' this one! Excited for the potential addition to the copy-code button!

I personally would like to still be able to view icons when is_demo is false. Let me know if you disagree!

Testing checklist

Icons in buttons should be visually centered.
Button stories have a control to toggle icons.
- If Include icon is true, show a dropdown of icons.
- Icons should update on select.
There should be no html:build errors.

packages/usa-button/src/usa-button.twig

amyleadem

@mejiaj I really like how you set up the Storybook controls here. The conditional control is great and importing the icon data was a really nice touch.

I added a couple of notes in the comments. Mostly, I am curious if it is possible to use flexbox for button alignment. I was looking at #5324 about text wrapping in button groups at the same time and it made me curious if there was a solution that would work for both issues. I added more detail in the comments.

There is a chance I am missing something, so I'm curious to hear what you think. Please let me know if you have any questions.

amyleadem · 2023-07-24T17:59:26Z

packages/usa-button/src/usa-button.stories.js

+        options: iconNames,
+        defaultValue: "add_circle_outline",
+      },
+      if: { arg: "icon_included" },


Nice! Love a conditional Storybook control. Cool find.

packages/usa-button/src/usa-button.stories.js

amyleadem · 2023-07-26T16:39:35Z

packages/usa-button/src/styles/_usa-button.scss

+  // @TODO: Use `:has()` selector once browser support improves.
+  // https://developer.mozilla.org/en-US/docs/Web/CSS/:has#browser_compatibility
+  .usa-icon {
+    margin-top: -0.1em; // Similar pattern to _usa-breadcrumb.scss:14.
+    vertical-align: middle;
+  }
+


What do you think about using flexbox for alignment on usa-button elements instead of creating a rule with negative margins here? I’d have to do some testing to confirm, but a quick experiment showed that it created more consistently centered elements at varying font and icon sizes, as well as allowed for buttons to be the same height when placed next to each other. It also didn't seem to impact visual display if a single button had no icon added.

For example, in this PR buttons in a segmented button group do not have a consistent height if icons are added to only some buttons in a row.

Additionally, I was looking at the open PR for button group wrap alignment, and it got me thinking that display: inline-flex and align-items: stretch might offer the answer to both scenarios.

I just did some quick poking and it looks like adding the style rules below would result in the following. Curious to hear what you think.

.usa-button { align-items: center; justify-content: center; display: inline-flex; flex-direction: row; } .usa-button-group{ align-items: stretch; } .usa-button-group__item { display: inline-flex; } .usa-icon { min-width: $icon-inline-size-magic-number; }

For reference, the HTML I used:

<ul class="usa-button-group usa-button-group--segmented"> <li class="usa-button-group__item"> <button type="button" class="usa-button"><svg class="usa-icon" aria-hidden="true" focusable="false" role="img"> <use xlink:href="./img/sprite.svg#navigate_before"></use> </svg>Previous</button> </li> <li class="usa-button-group__item"> <button type="button" class="usa-button usa-button--outline">Current</button> </li> <li class="usa-button-group__item"> <button type="button" class="usa-button usa-button--outline">Next<svg class="usa-icon" aria-hidden="true" focusable="false" role="img"> <use xlink:href="./img/sprite.svg#navigate_next"></use> </svg></button> </li> </ul> <ul class="usa-button-group usa-button-group--segmented"> <li class="usa-button-group__item"> <button type="button" class="usa-button"><svg class="usa-icon" aria-hidden="true" focusable="false" role="img"> <use xlink:href="./img/sprite.svg#navigate_before"></use> </svg>Previous</button> </li> <li class="usa-button-group__item"> <button type="button" class="usa-button usa-button--outline">Current</button> </li> <li class="usa-button-group__item"> <button type="button" class="usa-button usa-button--outline">Next with wrapping text<svg class="usa-icon" aria-hidden="true" focusable="false" role="img"> <use xlink:href="./img/sprite.svg#navigate_next"></use> </svg></button> </li> </ul> <ul class="usa-button-group usa-button-group--segmented"> <li class="usa-button-group__item"> <button type="button" class="usa-button font-body-xl">Map<svg class="usa-icon usa-icon--size-3" aria-hidden="true" focusable="false" role="img"> <use xlink:href="./img/sprite.svg#navigate_next"></use> </svg></button> </li> <li class="usa-button-group__item"> <button type="button" class="usa-button usa-button--outline">Hybrid</button> </li> <li class="usa-button-group__item"> <button type="button" class="usa-button usa-button--outline">Satellite<svg class="usa-icon usa-icon--size-4" aria-hidden="true" focusable="false" role="img"> <use xlink:href="./img/sprite.svg#navigate_next"></use> </svg></button> </li> </ul> <ul class="usa-button-group usa-button-group--segmented"> <li class="usa-button-group__item"> <button type="button" class="usa-button ">Map</button> </li> <li class="usa-button-group__item"> <button type="button" class="usa-button ">Hybrid</button> </li> <li class="usa-button-group__item"> <button type="button" class="usa-button ">Satellite</button> </li> </ul>

@amyleadem

Re: display: inline-flex. Because this is such a widely used button ― I wanted to minimize any potentially breaking changes as much as possible.

Since we already specify inline-block, I don't think the switch to inline-flex is as drastic as I imagined. Double-checking MDN display - CSS: Cascading Style Sheets | MDN and there should be very little display difference at all.

This addition improves the experience, but might require a new theme setting for the column gap. I'll play with this.

I've added flex box and a new column gap setting in 03082ea. For now, items are centered. I'd like to keep the button group changes separate, if possible. This would keep the scope of our testing more manageable.

Example with icons on both sides (for testing only)

Small screens

packages/usa-button/src/usa-button.twig

…button-icons

Co-authored-by: Amy Leadem <[email protected]>

Includes new setting $theme-button-column-gap, which takes units for the inner spacing.

…button-icons

Ensure type is passed & fix aria-disabled on big variant.

packages/uswds-core/src/styles/mixins/general/button-unstyled.scss

mahoneycm

Looks good to me! One quick question about the PR description:

Is this note from your PR still correct? Seems like it was from your previous implementation

Why this approach was chosen

Follows a similar pattern to what's been done in breadcrumb:14 & external link:32-40. A small enhancement seemed less heavy handed than other solution.

How I implemented

Added vertical alignment and negative margin to visually center.

Review checklist

packages/uswds-core/src/styles/mixins/general/button-unstyled.scss

mejiaj · 2023-09-12T13:53:44Z

@mahoneycm thanks for the review. I've responded to questions & updated PR description ― good catch!

amyleadem

This is looking good! I had just a few notes:

I like the new setting but was initially confused about its purpose based solely on the name. Wondering if it might be clearer to name$theme-button-column-gap something like $theme-button-icon-gap
In the card component, I noticed that the text was aligning to the left in the button when the line broke (Details in comment below).
Some PR description things:
- In the PR description, the control is referenced as “Icon included” but in storybook it is called “Add icon”
- I wonder if we should flag that this as “potentially breaking” breaking because it might adjust visual presentation if users have already added inline svgs to their buttons

Here are the things I tested in Chrome, Safari, and Firefox:

packages/usa-button/src/styles/_usa-button.scss

mahoneycm · 2023-09-14T14:13:03Z

I like @amyleadem's idea of renaming the setting from $theme-button-column-gap to something like $theme-button-icon-gap. It's a bit more descriptive which I appreciate!

- Renamed from `$theme-button-column-gap → $theme-button-icon-gap` for clarity. - Sorted button settings in alphabetical order.

mejiaj · 2023-09-25T17:51:54Z

@amyleadem @mahoneycm appreciate the suggestions. I've made the following changes:

Renamed icon setting to $theme-button-column-gap → $theme-button-icon-gap.
Update PR description.
Added text-align: center back to button.
Internal: Renamed storybook control icon_included → add_icon.

amyleadem

Looks good to me!

All updates look good. I also was able to successfully test both the visuals and settings configuration for $theme-button-icon-gap in uswds-sandbox.

<button class="usa-button" type="button">
    Default
    <svg class="usa-icon" aria-hidden="true" focusable="false" role="img">
        <use xlink:href="/assets/img/sprite.svg#add"></use>
      </svg>
</button>

$theme-button-icon-gap: 1

$theme-button-icon-gap: 3

amyleadem · 2023-09-26T17:40:46Z

@mejiaj One more question - should we consider this a potentially breaking change since users might have already added custom/conflicting styles to add icons? Probably worth warning them to confirm that their buttons with icons display as expected.

mahoneycm

Review checklist

mejiaj · 2023-10-11T19:00:40Z

@mejiaj One more question - should we consider this a potentially breaking change since users might have already added custom/conflicting styles to add icons? Probably worth warning them to confirm that their buttons with icons display as expected.

Undecided on this. I don't feel like the impact is that big, but open to suggestions.

It would be worth flagging that it's now supported so users can remove their custom styles. But labelling it Potentially breaking seems heavy.

thisisdano

Good update

James Mejia added 2 commits July 20, 2023 09:43

Add basic support for usa-icon in buttons

7fa6bda

Improve label for icon name control

110559a

mejiaj marked this pull request as ready for review July 21, 2023 15:39

mejiaj requested review from amyleadem and mahoneycm July 21, 2023 15:39

mejiaj modified the milestone: uswds 3.5.1 Jul 21, 2023

This was referenced Jul 21, 2023

USWDS-Site - Changelog: Add entry for Button icon alignment uswds/uswds-site#2207

Merged

Button/Icon: Improve default appearance of button with icon #4493

Closed

mahoneycm suggested changes Jul 21, 2023

View reviewed changes

packages/usa-button/src/usa-button.twig Outdated Show resolved Hide resolved

packages/usa-button/src/usa-button.twig Outdated Show resolved Hide resolved

amyleadem suggested changes Jul 26, 2023

View reviewed changes

mejiaj marked this pull request as draft August 7, 2023 19:43

James Mejia and others added 10 commits September 7, 2023 15:53

Merge branch 'develop' of github.com:uswds/uswds into jm-enhancement-…

c32391d

…button-icons

Update button story labels

eb23a05

Create a macro to fix display inconsistencies

87fc766

Update packages/usa-button/src/usa-button.stories.js

aacb83b

Co-authored-by: Amy Leadem <[email protected]>

Add note about duplicate icon name in icon control

3c3684e

Use flexbox for button alignment

03082ea

Includes new setting $theme-button-column-gap, which takes units for the inner spacing.

Merge branch 'develop' of github.com:uswds/uswds into jm-enhancement-…

558ba4f

…button-icons

Use justify-content in favor of text align

e943206

Reset unstyled button content

77d9316

Fix button attributes

b71814e

Ensure type is passed & fix aria-disabled on big variant.

mejiaj requested review from mahoneycm and amyleadem September 11, 2023 14:20

mejiaj marked this pull request as ready for review September 11, 2023 14:20

mejiaj requested a review from thisisdano September 11, 2023 14:20

Fix regression in header's nav link buttons

e157121

mejiaj commented Sep 11, 2023

View reviewed changes

packages/uswds-core/src/styles/mixins/general/button-unstyled.scss Show resolved Hide resolved

mahoneycm suggested changes Sep 11, 2023

View reviewed changes

packages/uswds-core/src/styles/mixins/general/button-unstyled.scss Show resolved Hide resolved

mejiaj requested a review from mahoneycm September 12, 2023 13:53

amyleadem suggested changes Sep 13, 2023

View reviewed changes

packages/usa-button/src/styles/_usa-button.scss Show resolved Hide resolved

James Mejia added 3 commits September 25, 2023 12:28

Rename button gap setting

a5854af

- Renamed from `$theme-button-column-gap → $theme-button-icon-gap` for clarity. - Sorted button settings in alphabetical order.

Add centered text back into buttons

fe2eaea

Update name of icon setting in storybookJS

94cd0f4

mejiaj requested a review from amyleadem September 25, 2023 17:49

amyleadem approved these changes Sep 25, 2023

View reviewed changes

mahoneycm approved these changes Sep 26, 2023

View reviewed changes

amyleadem mentioned this pull request Dec 4, 2023

USWDS-Site: Documentation updates for release 3.7.1 uswds/uswds-site#2385

Merged

Merge branch 'develop' into jm-enhancement-button-icons

b630276

mejiaj mentioned this pull request Dec 6, 2023

USWDS - Button Group: Improve appearance of buttons with wrapping text #5657

Merged

mahoneycm mentioned this pull request Feb 1, 2024

USWDS-Site: Documentation updates for release 3.8.0 uswds/uswds-site#2477

Closed

5 tasks

aduth mentioned this pull request Feb 7, 2024

USWDS - Tooltip: Fix handling of children icons #5263

Open

thisisdano approved these changes Feb 26, 2024

View reviewed changes

thisisdano merged commit 3cbb8d8 into develop Feb 26, 2024

thisisdano deleted the jm-enhancement-button-icons branch February 26, 2024 23:44

This was referenced Feb 28, 2024

USWDS-Site: Documentation updates for release 3.8.0 uswds/uswds-site#2509

Merged

USWDS: Release 3.8.0 tasks #5797

Closed

thisisdano mentioned this pull request Mar 11, 2024

USWDS 3.8.0 #5812

Merged

TomElliottFlexion mentioned this pull request Mar 25, 2024

10322: BUG: Icon spacing ustaxcourt/ef-cms#4779

Closed

SherfeyInv mentioned this pull request Jun 7, 2024

[Snyk] Upgrade @uswds/uswds from 3.7.1 to 3.8.0 SherfeyInv/digitalgov.gov#4

Closed

lane-formio mentioned this pull request Jun 22, 2024

[Snyk] Upgrade @uswds/uswds from 3.8.0 to 3.8.1 formio/uswds-viewer#21

Closed

sennhann29 mentioned this pull request Jul 26, 2024

[Snyk] Upgrade @uswds/uswds from 3.4.1 to 3.8.1 sennhann29/digitalgov.gov#4

Open

lane-formio mentioned this pull request Aug 31, 2024

[Snyk] Upgrade @uswds/uswds from 3.8.0 to 3.8.2 formio/uswds-viewer#25

Merged

FuhuXia mentioned this pull request Sep 26, 2024

[Snyk] Upgrade @uswds/uswds from 3.8.0 to 3.8.2 GSA/datagov-harvester#101

Merged

Mwindo mentioned this pull request Dec 16, 2024

Upgrade USWDS to 3.8.0 flexion/ef-cms#10333

Closed

USWDS - Button: Add basic support for usa-icon in buttons #5398

USWDS - Button: Add basic support for usa-icon in buttons #5398

Uh oh!

Conversation

mejiaj commented Jul 20, 2023 • edited by amyleadem Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Breaking change

Related issue

Related pull requests

Preview links

Problem statement

Solution

Why this approach was chosen

How I implemented

Alternatives

Major changes

Testing and review

Icon alignment

Storybook Controls

Testing checklist

Uh oh!

mahoneycm left a comment

Choose a reason for hiding this comment

Testing checklist

Uh oh!

Uh oh!

Uh oh!

amyleadem left a comment

Choose a reason for hiding this comment

Uh oh!

amyleadem Jul 24, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

amyleadem Jul 26, 2023

Choose a reason for hiding this comment

Uh oh!

mejiaj Sep 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mahoneycm left a comment

Choose a reason for hiding this comment

Why this approach was chosen

How I implemented

Review checklist

Uh oh!

Uh oh!

mejiaj commented Sep 12, 2023

Uh oh!

amyleadem left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mahoneycm commented Sep 14, 2023

Uh oh!

mejiaj commented Sep 25, 2023

Uh oh!

amyleadem left a comment

Choose a reason for hiding this comment

Uh oh!

amyleadem commented Sep 26, 2023

Uh oh!

mahoneycm left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review checklist

Uh oh!

mejiaj commented Oct 11, 2023

Uh oh!

thisisdano left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mejiaj commented Jul 20, 2023 •

edited by amyleadem

Loading

mejiaj Sep 8, 2023 •

edited

Loading

mahoneycm left a comment •

edited

Loading