Skip to content

Commit 023efe3

Browse files
authored
feat: simplify fallback of ui icon size numbers (#3571)
* feat(icon): simplify fallback of ui icon size numbers If a UI icon was not provided with its full name including sizing number (e.g. Dash100), the template would try and guess at a sizing number based on t-shirt size. This has been extracted out to a utility function and simplified, as this had gotten too complex, with too many exceptions. It no longer strips out any sizing numbers that were specifically provided. This also improves the logging of missing icons. * feat(icon): display placeholder icon when workflow icon is missing Display the design recommended "Circle" icon as a placeholder icon when the workflow icon cannot be found.This helps with identifying missing icons in VRTs, so the icon continues to take up the space it should, rather than disappearing entirely in some cases. This change also updates the templates of a few components to make sure they are rendering the correct UI icon: - fix(menu): update ui icon sizes used in template - docs(button): replace deprecated ui icon size in test - feat(swatch): use spec defined ui icon size number for dash - fix: icon set arg was missing a value in a few places - fix(actionbutton): define specific sizing for corner triangle - fix(menu): add missing context to menu templates to fix warning
1 parent 9ee29fd commit 023efe3

File tree

11 files changed

+127
-141
lines changed

11 files changed

+127
-141
lines changed

components/actionbutton/stories/template.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,13 @@ export const Template = ({
109109
${when(hasPopup && hasPopup !== "false", () =>
110110
Icon({
111111
size,
112-
iconName: "CornerTriangle",
112+
iconName: "CornerTriangle" + ({
113+
xs: "75",
114+
s: "75",
115+
m: "100",
116+
l: "200",
117+
xl: "300",
118+
}[size] || "100"),
113119
setName: "ui",
114120
customClasses: [`${rootClass}-hold`],
115121
}, context)

components/button/stories/button.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ const ButtonIconGroup = (args, context) => Container({
6060
testHeading: "UI icon (larger)",
6161
content: Template({
6262
...args,
63-
// UI icon that is larger than workflow sizing:
64-
iconName: "ArrowDown600",
63+
// Largest UI icon from UI icon set:
64+
iconName: "Cross600",
6565
iconSet: "ui",
6666
}, context),
6767
},

components/icon/stories/icon.mdx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ with:
6767

6868
<Canvas of={IconStories.UIArrows} />
6969

70+
## Missing workflow icon placeholder
71+
72+
In Storybook documentation, if a workflow icon name does not exist in the set, the placeholder "Circle" icon
73+
will be shown. Missing ui icons will render nothing. The following example purposefully uses an
74+
icon name that does not exist to demonstrate this behavior.
75+
76+
<Canvas of={IconStories.MissingWorkflowIcon} />
77+
7078
## Repositories for the icon SVG files
7179

7280
The UI icon SVGs are within the Spectrum CSS repository, which has its own package published to NPM:

components/icon/stories/icon.stories.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,3 +193,20 @@ UIArrows.tags = ["!dev"];
193193
UIArrows.parameters = {
194194
chromatic: { disableSnapshot: true },
195195
};
196+
197+
/**
198+
* In Storybook documentation, if a workflow icon name does not exist in the set, the
199+
* placeholder "Circle" icon will be shown. Missing ui icons will render
200+
* nothing. The following example purposefully uses an icon name that does
201+
* not exist to demonstrate this behavior.
202+
*/
203+
export const MissingWorkflowIcon = Default.bind({});
204+
MissingWorkflowIcon.storyName = "Missing workflow icon placeholder";
205+
MissingWorkflowIcon.tags = ["!dev"];
206+
MissingWorkflowIcon.args = {
207+
setName: "workflow",
208+
iconName: "ThisIconNameDoesNotExist",
209+
};
210+
MissingWorkflowIcon.parameters = {
211+
chromatic: { disableSnapshot: true },
212+
};

components/icon/stories/template.js

Lines changed: 41 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { ifDefined } from "lit/directives/if-defined.js";
66
import { styleMap } from "lit/directives/style-map.js";
77
import { unsafeSVG } from "lit/directives/unsafe-svg.js";
88
import { when } from "lit/directives/when.js";
9-
import { getSpriteSheetName, uiIconsCleaned, uiIconsWithDirections, workflowIconsCleaned } from "./utilities.js";
9+
import { appendUiIconDefaultSizing, getSpriteSheetName, uiIconsWithDirections, workflowIconsCleaned } from "./utilities.js";
1010

1111
import "../index.css";
1212

@@ -33,7 +33,7 @@ import "../index.css";
3333
export const Template = ({
3434
rootClass = "spectrum-Icon",
3535
size = "m",
36-
setName,
36+
setName = "workflow",
3737
iconName,
3838
uiIconName,
3939
fill,
@@ -50,113 +50,53 @@ export const Template = ({
5050
iconName = uiIconName;
5151
}
5252

53-
// Make sure icon name is provided.
54-
if (!iconName) {
53+
// Make sure icon set is provided.
54+
if (!["ui","workflow"].includes(setName)) {
5555
console.warn(
56-
"Icon: Could not render a result because no icon name was provided to the icon template."
56+
`Icon "${iconName}" is missing its icon set. Make sure you are explicitly setting either the workflow or ui icon set.`
5757
);
5858
return html``;
5959
}
6060

61-
// Name of icon that corresponds with SVG file. This may differ from the icon name, such as with
62-
// directional icons that use a single icon.
63-
let idKey = iconName;
64-
65-
// If a descriptor like "Right", "Left", "Down", or "Up" is present for the UI icons Chevron or
66-
// Arrow, use that only for the class name and not the icon fetch. This is because these use a
67-
// single icon file that is rotated in CSS.
68-
if (
69-
["Right", "Left", "Down", "Up"].some((c) => idKey.includes(c)) &&
70-
setName === "ui"
71-
) {
72-
idKey = idKey.replace(/(Right|Left|Down|Up)/, "");
73-
}
74-
75-
// Make sure icon set is provided.
76-
if (!setName) {
77-
console.warn(
78-
`Icon "${idKey}" is missing its icon set. Make sure you are explicitly setting either the workflow or ui icon set.`
79-
);
61+
// Make sure icon name is provided.
62+
if (!iconName) {
63+
console.warn("Icon: Could not render a result because no icon name was provided to the icon template.");
8064
return html``;
8165
}
8266

8367
/**
84-
* Fallback UI Icon sizing number.
85-
*
86-
* If the icon name includes its scale, we want to leave that scale. This is preferred,
87-
* as UI icons do not use workflow icon sizing.
68+
* Append approximate sizing number to UI icons passed in without a sizing number.
8869
*
89-
* If the UI icon name does not include scale, or the scale does not exist in the current
90-
* list of UI icons, reformat it to approximate the provided sizing for the component.
70+
* Note: It's preferred for components to provide the specific UI sizing numbers in the UI icon
71+
* name, rather than relying on this approximation, as UI icons do not use t-shirt sizing.
9172
*/
92-
if (
93-
setName === "ui" &&
94-
(
95-
// Does not already have size number at the end.
96-
!idKey.match(/\d{2,3}$/) ||
97-
// If the provided icon name includes the sizing number, make sure it's a supported sizing number;
98-
// if not, strip it from the key.
99-
(
100-
idKey.match(/\d{2,3}$/) &&
101-
!uiIconsCleaned.includes(idKey)
102-
)
103-
)
104-
) {
105-
let sizeVal;
106-
switch (size) {
107-
case "xs":
108-
if (["CornerTriangle", "Cross"].some(c => idKey.startsWith(c))) {
109-
sizeVal = "75";
110-
}
111-
else if (["Arrow", "Asterisk", "LinkOut"].some(c => idKey.startsWith(c))) {
112-
sizeVal = "100";
113-
}
114-
else {
115-
sizeVal = "50";
116-
}
117-
break;
118-
case "s":
119-
if (["Arrow", "Asterisk", "LinkOut"].some(c => idKey.startsWith(c))) {
120-
sizeVal = "100";
121-
}
122-
else {
123-
sizeVal = "75";
124-
}
125-
break;
126-
case "l":
127-
if (["Arrow"].some(c => idKey.startsWith(c))) {
128-
sizeVal = "400";
129-
}
130-
else {
131-
sizeVal = "200";
132-
}
133-
break;
134-
case "xl":
135-
case "xxl":
136-
if (["Arrow"].some(c => idKey.startsWith(c))) {
137-
sizeVal = "400";
138-
}
139-
else {
140-
sizeVal = "300";
141-
}
142-
break;
143-
default:
144-
sizeVal = "100";
145-
break;
146-
}
73+
if (setName === "ui") {
74+
iconName = appendUiIconDefaultSizing(iconName, size);
75+
}
14776

148-
console.warn(`Using fallback UI Icon sizing number "${sizeVal}" for "${idKey}". UI icon size was not provided or does not exist in the list of available UI icons.`);
77+
// Make sure icon exists in the set.
78+
if (setName == "ui" && !uiIconsWithDirections.includes(iconName)) {
79+
console.warn(`Icon: Could not render an icon with the name "${iconName}" because it does not exist in the "ui" icon set.`);
80+
return html``;
81+
}
14982

150-
// Replace sizing number on idKey and iconName with new fallback size.
151-
idKey = idKey.replace(/\d{2,3}$/, "");
152-
idKey += sizeVal;
83+
if (setName == "workflow" && !workflowIconsCleaned.includes(iconName)) {
84+
console.warn(`Icon: Could not render the correct icon with the name "${iconName}" because it does not exist in the "workflow" icon set. Rendering the placeholder icon instead.`);
85+
iconName = "Circle";
86+
}
15387

154-
iconName = iconName.replace(/\d{2,3}$/, "");
155-
iconName += sizeVal;
88+
// Name of icon that corresponds with SVG file. This may differ from the icon name, such as with
89+
// directional icons that use a single icon.
90+
let iconNameToLoad = iconName;
15691

157-
if (!uiIconsCleaned.includes(idKey)) {
158-
console.error(`The UI icon "${idKey}" does not exist in the list of available UI icons.`);
159-
}
92+
// If a descriptor like "Right", "Left", "Down", or "Up" is present for the UI icons Chevron or
93+
// Arrow, use that only for the class name and not the icon fetch. This is because these use a
94+
// single icon file that is rotated in CSS.
95+
if (
96+
["Right", "Left", "Down", "Up"].some((c) => iconNameToLoad.includes(c)) &&
97+
setName === "ui"
98+
) {
99+
iconNameToLoad = iconNameToLoad.replace(/(Right|Left|Down|Up)/, "");
160100
}
161101

162102
/**
@@ -178,8 +118,8 @@ export const Template = ({
178118
*/
179119
if (!useRef) {
180120
let svgString;
181-
if (loaded?.icons && loaded?.icons[setName]?.[idKey]) {
182-
svgString = loaded.icons[setName][idKey];
121+
if (loaded?.icons && loaded?.icons[setName]?.[iconNameToLoad]) {
122+
svgString = loaded.icons[setName][iconNameToLoad];
183123
}
184124

185125
// Return the individual SVG's entire markup.
@@ -194,12 +134,12 @@ export const Template = ({
194134
)}`;
195135
}
196136
else {
197-
console.warn(`Could not find SVG markup for "${idKey}" in context.loaded.icons. Did you pass through context? Falling back to using the sprite sheet reference instead.`);
137+
console.warn(`Could not find SVG markup for "${iconNameToLoad}" in context.loaded.icons. Was context passed through in the template? Falling back to using the sprite sheet reference instead.`);
198138
}
199139
}
200140

201141
// ID of the icon within the sprite sheet for its icon set.
202-
const iconID = getSpriteSheetName(idKey, setName);
142+
const iconID = getSpriteSheetName(iconNameToLoad, setName);
203143

204144
// Return SVG markup with a reference to the icon ID within the sprite sheet.
205145
return html`<svg
@@ -211,7 +151,7 @@ export const Template = ({
211151
aria-label=${iconName}
212152
role="img"
213153
>
214-
<title id=${idKey}>${idKey.replace(/([A-Z])/g, " $1").trim()}</title>
154+
<title id=${iconNameToLoad}>${iconNameToLoad.replace(/([A-Z])/g, " $1").trim()}</title>
215155
<use xlink:href="#${iconID}" href="#${iconID}" />
216156
</svg>`;
217157
};
@@ -304,7 +244,7 @@ export const WorkflowDefaultTemplate = (args, context) => html`
304244
},
305245
context,
306246
[
307-
"AlertCircle",
247+
"AlertTriangle",
308248
"Bell",
309249
"Camera",
310250
"Color",
@@ -315,7 +255,7 @@ export const WorkflowDefaultTemplate = (args, context) => html`
315255
"Files",
316256
"Hand",
317257
"Lightbulb",
318-
"Paragraph",
258+
"InfoCircle",
319259
]
320260
)}
321261
`;

components/icon/stories/utilities.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,3 +190,27 @@ export const getUiIconSizes = (uiIcons) => {
190190
};
191191

192192
export const uiIconSizes = getUiIconSizes(uiIconsWithDirections);
193+
194+
/**
195+
* If UI icon name does not have a sizing number appended, add one to approximate the provided
196+
* t-shirt sizing for the component, based on the most common mapping.
197+
*
198+
* @param {string} uiIconName
199+
* @param {string} size t-shirt sizing
200+
* @returns {string} uiIconName with appended default sizing number, if one is not already present.
201+
*/
202+
export const appendUiIconDefaultSizing = (uiIconName, size = "m") => {
203+
// If icon name already has a size number on the end, no change is needed.
204+
if (uiIconName.match(/\d{2,3}$/)) {
205+
return uiIconName;
206+
}
207+
208+
return uiIconName + ({
209+
xs: "50",
210+
s: "75",
211+
m: "100",
212+
l: "200",
213+
xl: "300",
214+
xxl: "400",
215+
}[size] || "100");
216+
};

0 commit comments

Comments
 (0)