Skip to content

Commit 570d19f

Browse files
jawinncastastrophe
jawinn
authored and
castastrophe
committed
fix(button): support any existing use of ui icons
Support any existing use of ui icons with the updated wrapping behavior. And add Chromatic only testing of them to the Wrapping story. Workflow icons are intended, with the use of the spectrum-component-top-to-workflow-icon tokens, but UI icons have not yet been specifically excluded in guidelines and are currently in use within SplitButton in this library. This keeps UI icons that are smaller than the intended workflow icon, better vertically centered with the text within the button.
1 parent b7fd783 commit 570d19f

File tree

3 files changed

+74
-23
lines changed

3 files changed

+74
-23
lines changed

‎components/button/index.css

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ governing permissions and limitations under the License.
2323
--spectrum-button-focus-ring-border-radius: calc(var(--spectrum-button-border-radius) + var(--spectrum-button-focus-ring-gap));
2424
--spectrum-button-focus-ring-thickness: var(--spectrum-focus-indicator-thickness);
2525
--spectrum-button-focus-indicator-color: var(--spectrum-focus-indicator-color);
26+
--spectrum-button-intended-icon-size: var(--spectrum-workflow-icon-size-50);
2627
}
2728

2829
.spectrum-Button--sizeS {
@@ -40,6 +41,7 @@ governing permissions and limitations under the License.
4041
--spectrum-button-top-to-text: var(--spectrum-button-top-to-text-small);
4142
--spectrum-button-bottom-to-text: var(--spectrum-button-bottom-to-text-small);
4243
--spectrum-button-top-to-icon: var(--spectrum-component-top-to-workflow-icon-75);
44+
--spectrum-button-intended-icon-size: var(--spectrum-workflow-icon-size-75);
4345
}
4446

4547
.spectrum-Button--sizeM {
@@ -57,6 +59,7 @@ governing permissions and limitations under the License.
5759
--spectrum-button-top-to-text: var(--spectrum-button-top-to-text-medium);
5860
--spectrum-button-bottom-to-text: var(--spectrum-button-bottom-to-text-medium);
5961
--spectrum-button-top-to-icon: var(--spectrum-component-top-to-workflow-icon-100);
62+
--spectrum-button-intended-icon-size: var(--spectrum-workflow-icon-size-100);
6063

6164
&[pending],
6265
&.is-pending {
@@ -79,6 +82,7 @@ governing permissions and limitations under the License.
7982
--spectrum-button-top-to-text: var(--spectrum-button-top-to-text-large);
8083
--spectrum-button-bottom-to-text: var(--spectrum-button-bottom-to-text-large);
8184
--spectrum-button-top-to-icon: var(--spectrum-component-top-to-workflow-icon-200);
85+
--spectrum-button-intended-icon-size: var(--spectrum-workflow-icon-size-200);
8286

8387
&[pending],
8488
&.is-pending {
@@ -101,6 +105,7 @@ governing permissions and limitations under the License.
101105
--spectrum-button-top-to-text: var(--spectrum-button-top-to-text-extra-large);
102106
--spectrum-button-bottom-to-text: var(--spectrum-button-bottom-to-text-extra-large);
103107
--spectrum-button-top-to-icon: var(--spectrum-component-top-to-workflow-icon-300);
108+
--spectrum-button-intended-icon-size: var(--spectrum-workflow-icon-size-300);
104109

105110
.spectrum--medium &[pending],
106111
.spectrum--medium &.is-pending {
@@ -143,10 +148,19 @@ governing permissions and limitations under the License.
143148
}
144149

145150
.spectrum-Icon {
151+
/* Any block-size difference between the intended workflow icon size and actual icon used.
152+
Helps support any existing use of smaller UI icons instead of intended Workflow icons. */
153+
--_icon-size-difference: max(0px,
154+
var(--spectrum-button-intended-icon-size) -
155+
var(--spectrum-icon-block-size, var(--spectrum-button-intended-icon-size))
156+
);
157+
146158
margin-block-start: max(0px,
147159
var(--mod-button-top-to-icon, var(--spectrum-button-top-to-icon)) -
148-
var(--mod-button-border-width, var(--spectrum-button-border-width))
160+
var(--mod-button-border-width, var(--spectrum-button-border-width)) +
161+
(var(--_icon-size-difference, 0px) / 2)
149162
);
163+
150164
margin-inline-start: calc(
151165
var(--mod-button-edge-to-visual, var(--spectrum-button-edge-to-visual)) -
152166
var(--mod-button-edge-to-text, var(--spectrum-button-edge-to-text))

‎components/button/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
},
1616
"main": "dist/index-vars.css",
1717
"peerDependencies": {
18-
"@spectrum-css/icon": ">=4",
18+
"@spectrum-css/icon": ">=6",
1919
"@spectrum-css/progresscircle": ">=2",
2020
"@spectrum-css/tokens": ">=13"
2121
},

‎components/button/stories/button.stories.js

Lines changed: 58 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { html } from "lit";
22
import { ifDefined } from "lit/directives/if-defined.js";
33
import { styleMap } from "lit/directives/style-map.js";
4+
import { when } from "lit/directives/when.js";
45

6+
import { default as IconStories } from "@spectrum-css/icon/stories/icon.stories.js";
57
import { Template as Typography } from "@spectrum-css/typography/stories/template.js";
68
import { Template } from "./template";
79

8-
import { default as IconStories } from "@spectrum-css/icon/stories/icon.stories.js";
9-
1010
export default {
1111
title: "Components/Button",
1212
description:
@@ -131,6 +131,18 @@ export default {
131131
},
132132
};
133133

134+
/**
135+
* Optional wrapper for each button used within other templates, to assist with the "stacked"
136+
* layout and the testing of wrapping text.
137+
*/
138+
const ButtonWrap = (layout, content) => {
139+
const buttonWrapStyles = {
140+
'margin-block': '15px',
141+
'max-width': '480px',
142+
};
143+
return layout === "stacked" ? html`<div style=${styleMap(buttonWrapStyles)}>${content}</div>` : content;
144+
};
145+
134146
/**
135147
* Multiple button variations displayed in one story template.
136148
* Used as the base template for the stories.
@@ -143,15 +155,6 @@ const CustomButton = ({
143155
customStyles = {},
144156
...args
145157
}) => {
146-
// Optional wrapper for each button, to assist with the testing of wrapping text.
147-
const ButtonWrap = (content) => {
148-
const buttonWrapStyles = {
149-
'margin-block': '15px',
150-
'max-width': '480px',
151-
};
152-
return layout === "stacked" ? html`<div style=${styleMap(buttonWrapStyles)}>${content}</div>` : content;
153-
};
154-
155158
return html`
156159
<div
157160
style=${ifDefined(styleMap({
@@ -160,28 +163,30 @@ const CustomButton = ({
160163
...customStyles
161164
}))}
162165
>
163-
${ButtonWrap(Template({
166+
${ButtonWrap(layout, Template({
164167
...args,
165168
staticColor,
166169
iconName: undefined,
167170
}))}
168-
${ButtonWrap(Template({
171+
${ButtonWrap(layout, Template({
169172
...args,
170173
staticColor,
171174
iconName: undefined,
172175
treatment: "outline",
173176
}))}
174-
${ButtonWrap(Template({
177+
${ButtonWrap(layout, Template({
175178
...args,
176179
staticColor,
177180
iconName: iconName ?? "Edit",
178181
}))}
179-
${showIconOnlyButton ? ButtonWrap(Template({
180-
...args,
181-
staticColor,
182-
hideLabel: true,
183-
iconName: iconName ?? "Edit",
184-
})) : ''}
182+
${when(showIconOnlyButton, () =>
183+
ButtonWrap(layout, Template({
184+
...args,
185+
staticColor,
186+
hideLabel: true,
187+
iconName: iconName ?? "Edit",
188+
}))
189+
)}
185190
</div>
186191
`;
187192
};
@@ -255,6 +260,37 @@ const ButtonsWithForcedColors = ({
255260
</div>
256261
`;
257262

263+
/**
264+
* Wrapping story template, displaying some additional variants for Chromatic.
265+
*/
266+
const WrappingTemplate = ({layout, ...args}) => {
267+
if (window.isChromatic()) {
268+
return html`
269+
${CustomButton({layout, ...args})}
270+
<div style=${ifDefined(styleMap({ padding: "1rem" }))}>
271+
${ButtonWrap(layout, Template({
272+
...args,
273+
iconName: "Edit",
274+
treatment: "outline",
275+
}))}
276+
${ButtonWrap(layout, Template({
277+
...args,
278+
// Uses a UI icon that is smaller than workflow sizing, to test alignment:
279+
iconName: "Cross100",
280+
}))}
281+
${ButtonWrap(layout, Template({
282+
...args,
283+
// UI icon that is larger than workflow sizing:
284+
iconName: "ArrowDown600",
285+
treatment: "outline",
286+
}))}
287+
</div>
288+
`;
289+
}
290+
// Otherwise use the default template.
291+
return CustomButton({layout, ...args});
292+
};
293+
258294
export const Accent = CustomButton.bind({});
259295
Accent.args = {
260296
variant: "accent",
@@ -306,7 +342,8 @@ WithForcedColors.args = {
306342
iconName: "Actions",
307343
};
308344

309-
export const Wrapping = CustomButton.bind({});
345+
export const Wrapping = WrappingTemplate.bind({});
346+
Wrapping.storyName = "Wrapping";
310347
Wrapping.args = {
311348
layout: "stacked",
312349
showIconOnlyButton: false,

0 commit comments

Comments
 (0)