fix icon button positioning

[aciccarello]

Type: bug

The following has been addressed in the PR:

• [x] There is a related issue
• [x] All code matches the style guide
• [ ] Unit tests are included in the PR

Description: Vertically aligns the label of a button so that icons and text are aligned and adds a 8px gap between text and icons. Where the button variants overrode the label styles (mostly in Dojo theme), it was necessary to compose them back in. This uses flex gap which isn't currently supported by Safari (though it is in Technical Preview). I figured the style wasn't strictly necessary so a slight degradation seemed acceptable.

The example was updated to include more button types and to use small sized icons which fit better and should be recommended when used with the material theme.

Resolves #1594

[vercel[bot]]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

widget-test-docs – ./

🔍 Inspect: https://vercel.com/dojo/widget-test-docs/F4R2PAGSNKQx3DVt1nEf5NKhwSUQ
✅ Preview: https://widget-test-do-git-fork-aciccarello-1594-icon-button-pos-60cb2b.vercel.app

dojo.widgets – ./

🔍 Inspect: https://vercel.com/dojo/dojo.widgets/GeQX3McttSwzJtHxPpmParkh7zUy
✅ Preview: https://dojowidgets-git-fork-aciccarello-1594-icon-button-pos-7c4d85.vercel.app

[codecov[bot]]

Codecov Report

Merging #1681 (1bf4aad) into master (ebbad1b) will decrease coverage by 0.02%. The diff coverage is 89.65%.

@@            Coverage Diff             @@
##           master    #1681      +/-   ##
==========================================
- Coverage   89.98%   89.96%   -0.03%     
==========================================
  Files          94       94              
  Lines        5012     5031      +19     
  Branches     1354     1364      +10     
==========================================
+ Hits         4510     4526      +16     
- Misses        248      249       +1     
- Partials      254      256       +2     

Impacted Files	Coverage Δ
src/floating-action-button/index.tsx	100.00% <ø> (ø)
src/common/tests/support/test-helpers.ts	93.18% <66.66%> (-6.82%)	:arrow_down:
src/action-button/index.tsx	100.00% <100.00%> (ø)
src/button/index.tsx	83.63% <100.00%> (+3.63%)	:arrow_up:
src/outlined-button/index.tsx	100.00% <100.00%> (ø)
src/raised-button/index.tsx	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ebbad1b...1bf4aad. Read the comment docs.

[tomdye]

@aciccarello it looks like material has a specific icon-button with different classes used to align the label and icon appropriately - https://material.io/develop/web/components/buttons/icon-buttons

Do you think it might make sense to have a separate IconButton ? That way we could make the label optional and thus allow for buttons containing only an icon also. With this approach, in the material case, the appropriate icon-button css classes could be used.

Either way, including a means to supply both a label and an icon would give us the ability to wrap those nodes and use a class to add padding / margin without needing to use css which is not in safari yet.

[aciccarello]

@tomdye I'm confused how material integrates that with the different variants. On the design spec they show it being used with different variants. How do they have a single icon-button?

Is an icon-only icon a priority? If so I can work on that styling (which would be mostly reducing the button padding).

[aciccarello]

Ah, the icon-button is only for icon-only buttons. It's taller (48px instead of 36px) and square. For a button with a label we should stick with what we have. If we want an icon-only button I'd recommend adding an icon-button widget separately.

Note: For buttons with both icons and text, use the mdc-button component. For more information, see the mdc-buttondocs.

• Material Web Icon Button Description

I'm working on a margin solution which doesn't use flex gap and also supports the adjustments for the margin between the button border and the icon by passing in the icon separately.

[tomdye]

@aciccarello If we changed the button's children api to take icon and label, both of which were RenderResults then we could do the appropriate wrapping and also handle the case where only an icon was passed and thus handle the sizing etc correctly. Would mean a breaking change though unless we allowed either / or. ie.

<Button>Button Label</Button>
<Button>{{ label: 'button label' }}</Button>
<Button>{{ icon: <Icon type='add'/> }}</Button>
<Button>{{ label: 'button label', icon: <Icon type='add'/>}}</Button>

Might be worth having a discussion about in the dojo room. I think the breaking change is the better approach personally as we're moving onto dojo 8 and it's a simple enough change, but the fallback to the previous api may be acceptable.