Skip to content

Commit

Permalink
refactor(icon)!: tokens migration (#2347)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: migrate Icon from DNA tokens to Spectrum tokens.

Additionally:
* refactor(icon): migrate css to spectrum tokens

Migrates Icon CSS from using DNA/vars tokens to Spectrum tokens.
Refactors UI Icons to be a little cleaner and not need placeholders.

Icons now change the value of the property "--spectrum-icon-size" to
set their width and height. They also have three additional mods
available for setting the size (both width and height) or the individual
width and height.

* refactor(icon): remove legacy xvar css and plugin for combined icons

- Remove 'xvar' and 'x--' code within UI icon CSS, along with the build
  plugins that were used only for this. This was only only needed
  previously when the build did not allow 'var()' and '--property' here.

- Simplify and better document code used for combined UI icons and the
  medium/large platform scale. Remove old browser support here that is
  no longer needed with the browsers and features that are currently
  supported by the library. The old fallback to set display inline was
  pre Firefox version 56 [2017].

* refactor(icon): remove gripper icon classes

Removing the gripper icon classes as they were incorrect and not used,
and there are no tokens defined yet to set the actual classes to.

The gripper icon classes used previously were wrong in several ways.
For one, they were using '100' size naming in the classes which are not
currently used or displayed. These icons are without the number size.
The old alias values being applied to them also looked incorrect when
looking at the widths, and the CSS was swapping width for height.

That there is no size applied to these icons was obfuscated by the fact
that the attribute width="10" is being applied to icons in Storybook.

Note: SWC is currently showing these icons with workflow sizing.
These gripper icons do not have size tokens defined yet, but they may be
added in the future "as they are needed"; when these icons start being
used.

* docs(icon): storybook - add kitchen sink style story for chromatic

Cover the various types of icons in a Chromatic only story.
Covers different icon sets, sizes, and color in the VRTs.

* feat(icon): adjust icon sizing custom properties

Make sure we always have custom properties that contains the width and
height, that we can rely upon for CSS calculations. Regardless of
whether the individual dimensions are specified or just the size is
specified (that applies to both dimensions).

* fix(icon): storybook - remove inline width attribute

The icons in Storybook were adding an inline "width" attribute set to
10, which was previously obfuscating issues with sizing. Removes this
attribute and leaves sizing up to CSS.

* feat(icon): support for xs workflow icon size

Added extra small workflow icon size. This has a token, is defined on
some of the design redlines (Action Button), and is currently used in
the Contextual Help component, as seen in the VRT run.

* feat(icon): remove theme files without content

Recent updates to main make it no longer necessary to include empty
theme files for the build to work.

* feat(icon): storybook - use ui icon size numbers

Disables the size control for UI icons and adds each size number to the
list of available UI icon names in Storybook.

UI icons have specific sizing and don't use the t-shirt sizing that
Workflow icons do. They have more size numbers than there are t-shirt
sizes, so they can't be directly mapped to each other. The different
UI icons have different size numbers, so the size numbers can't use a
single control.

* feat(icon): storybook - show all ui icons in chromatic template

Show all UI icons, including number sizing, in the Chromatic template.
Condenses and improves some of the template logic.

* fix(icon): wrong workflow icons appearing for arrow and chevron

Fix bug where the wrong icon was being rendered for workflow arrow and
chevron. These are both icons with names that exist in both icon sets.
There was logic being applied to the workflow icons that should have
only have been applied to the UI icon.

* chore(icon): manual version bump for beta release

* feat(icon): add xxs size for migration and use renamed xxl property

Add XXS size to support existing SWC size. Uses the values from
--spectrum-global-dimension-size-150, as used in SWC's custom icon CSS.

Included comments to note that xxs and xxl are planned to be deprecated
in Spectrum 2, as they are not a part of the design spec.

* chore(icon): set current beta versions already released

* build(icon): minimum tokens version with xxl and xxs sizing

Update required tokens version with a minimum of the latest release that
includes the new custom-vars for the xxl and xxs workflow icon sizes.

---------

Co-authored-by: Patrick Fulton <pfulton@adobe.com>
  • Loading branch information
jawinn and pfulton authored Feb 5, 2024
1 parent 4d5e662 commit b63631a
Show file tree
Hide file tree
Showing 20 changed files with 382 additions and 562 deletions.
2 changes: 1 addition & 1 deletion components/icon/gulpfile.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@
module.exports = require("@spectrum-css/component-builder");
module.exports = require("@spectrum-css/component-builder-simple");
14 changes: 10 additions & 4 deletions components/icon/icons.css
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,28 @@ governing permissions and limitations under the License.

.spectrum-Icon,
.spectrum-UIIcon {
--spectrum-icon-inline-size: var(--mod-icon-inline-size, var(--mod-icon-size, var(--spectrum-icon-size)));
--spectrum-icon-block-size: var(--mod-icon-block-size, var(--mod-icon-size, var(--spectrum-icon-size)));

display: inline-block;
inline-size: var(--spectrum-icon-inline-size);
block-size: var(--spectrum-icon-block-size);

/* Use custom pass through or inherit the text color */
/* Use custom pass through or inherit the text color. */
color: var(--mod-icon-color, inherit);

/* Fill should match the current text color */
/* Fill should match the current text color. */
fill: currentColor;

/* Hide the svg overflow in IE. */
/* Hide the SVG overflow in IE. */
&:not(:root) {
overflow: hidden;
}

/* Don't catch clicks or hover, otherwise they may not escape the SVG */
/* Don't catch clicks or hover, otherwise they may not escape the SVG. */
pointer-events: none;
}

@media (forced-colors: active) {
.spectrum-Icon,
.spectrum-UIIcon {
Expand Down
11 changes: 8 additions & 3 deletions components/icon/metadata/mods.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
| Modifiable custom properties |
| ---------------------------- |
| `--mod-icon-color` |
| Modifiable custom properties |
| ------------------------------ |
| `--mod-icon-block-size` |
| `--mod-icon-color` |
| `--mod-icon-inline-size` |
| `--mod-icon-size` |
| `--mod-ui-icon-large-display` |
| `--mod-ui-icon-medium-display` |
6 changes: 3 additions & 3 deletions components/icon/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@spectrum-css/icon",
"version": "5.1.0",
"version": "6.0.0-beta.1",
"description": "The Spectrum CSS icon component",
"license": "Apache-2.0",
"author": "Adobe",
Expand All @@ -13,9 +13,9 @@
"bugs": {
"url": "https://github.com/adobe/spectrum-css/issues"
},
"main": "dist/index-vars.css",
"main": "dist/index.css",
"peerDependencies": {
"@spectrum-css/vars": ">=9"
"@spectrum-css/tokens": ">=13.1"
},
"publishConfig": {
"access": "public"
Expand Down
2 changes: 1 addition & 1 deletion components/icon/project.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$schema": "../../node_modules/nx/schemas/project-schema.json",
"name": "icon",
"tags": ["component", "legacy"],
"tags": ["component"],
"targets": {
"build": {},
"clean": {},
Expand Down
123 changes: 106 additions & 17 deletions components/icon/stories/icon.stories.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,27 @@
// Import the component markup template
import isChromatic from "chromatic/isChromatic";
import { html } from "lit";
import { styleMap } from "lit/directives/style-map.js";
import { when } from "lit/directives/when.js";

import { Template } from "./template";
import { uiIconSizes, uiIconsWithDirections, workflowIcons } from "./utilities.js";

import { uiIcons, workflowIcons } from "./utilities.js";
/**
* Create a list of all UI Icons with their sizing numbers.
*
* The list is a little long until Storybook adds a way to use conditional options
* in controls, e.g. a "uiSize" control with options pulled from uiIconSizes:
* @see https://github.com/storybookjs/storybook/discussions/24235
*/
const uiIconNameOptions = uiIconsWithDirections.map((iconName) => {
const baseIconName = iconName.replace(/(Left|Right|Up|Down)$/, '');
// Icons like Gripper that don't have sizes yet, represented by any empty array.
if (uiIconSizes[baseIconName]?.length == 0){
return [baseIconName];
}
return uiIconSizes[baseIconName]?.map(sizeNum => iconName + sizeNum) ?? [];
}).flat();

export default {
title: "Components/Icon",
Expand All @@ -13,14 +33,15 @@ export default {
express: { table: { disable: true } },
reducedMotion: { table: { disable: true } },
size: {
name: "Size",
name: "Workflow Icon Size",
type: { name: "string", required: true },
table: {
type: { summary: "string" },
category: "Component",
},
options: ["s", "m", "l", "xl", "xxl"],
options: ["xs", "s", "m", "l", "xl", "xxl"],
control: "select",
if: { arg: "setName", eq: "workflow" },
},
setName: {
name: "Icon set",
Expand Down Expand Up @@ -51,15 +72,7 @@ export default {
category: "Content",
},
options: [
...uiIcons.filter((c) => !["Chevron", "Arrow"].includes(c)),
"ArrowRight",
"ArrowLeft",
"ArrowUp",
"ArrowDown",
"ChevronRight",
"ChevronLeft",
"ChevronUp",
"ChevronDown",
...uiIconNameOptions,
],
control: "select",
if: { arg: "setName", eq: "ui" },
Expand Down Expand Up @@ -90,10 +103,86 @@ export default {
},
};

export const Default = (args) => Template({
...args,
iconName: args.iconName ?? args.uiIconName,
setName: args.setName ?? (args.uiIconName ? "ui" : "workflow"),
});
export const Default = (args) => {
if (isChromatic()){
return TestTemplate({ ...args });
} else {
return Template({
...args,
iconName: args.iconName ?? args.uiIconName,
setName: args.setName ?? (args.uiIconName ? "ui" : "workflow"),
});
}
}

Default.args = {};

/**
* Chromatic VRT template that displays multiple icons to cover various options.
*/
const TestTemplate = ({
staticColor,
customStyles = {},
...args
}) => {
const workflow_row_args = [
{
setName: "workflow",
iconName: "Alert",
fill: "var(--spectrum-negative-content-color-default)",
},
{
setName: "workflow",
iconName: "Hand",
},
{
setName: "workflow",
iconName: "Help",
},
{
setName: "workflow",
iconName: "ArrowLeft",
},
{
setName: "workflow",
iconName: "ArrowRight",
},
{
setName: "workflow",
iconName: "ChevronDown",
}
];

return html`
${workflow_row_args.map((row_args) => html`
<div
style=${styleMap({
display: "flex",
gap: "16px",
marginBottom: "16px",
})}
>
${['xs','s','m','l','xl','xxl'].map(
(size) => Template({ ...args, ...row_args, size })
)}
</div>`
)}
<div style="margin-top:32px;">
${uiIconsWithDirections.map(iconName => html`
<div
style=${styleMap({
display: "flex",
gap: "16px",
})}
>
${uiIconSizes[iconName.replace(/(Left|Right|Up|Down)$/, '')]?.map((iconSize) =>
Template({ ...args, setName: "ui", iconName: iconName + iconSize })
)}
${when(uiIconSizes[iconName]?.length == 0, () =>
Template({ ...args, setName: "ui", iconName })
)}
</div>`
)}
</div>
`;
};
66 changes: 42 additions & 24 deletions components/icon/stories/template.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { classMap } from "lit/directives/class-map.js";
import { ifDefined } from "lit/directives/if-defined.js";
import { unsafeSVG } from "lit/directives/unsafe-svg.js";

import { fetchIconSVG, uiIcons, workflowIcons } from "./utilities.js";
import { fetchIconSVG, uiIcons, uiIconSizes, workflowIcons } from "./utilities.js";

import "../index.css";

Expand All @@ -17,7 +17,7 @@ import "../index.css";
* @description Icon template that renders an icon based on the provided icon name and set name.
* @param {IconProps} props
* @param {string} props.rootClass
* @param {"s"|"m"|"l"|"xl"} props.size
* @param {"xs"|"s"|"m"|"l"|"xl"|"xxl"} props.size
* @param {"ui"|"workflow"} props.setName
* @param {string} props.iconName - Icon name with or without the icon scale number appended. Names with the scale (e.g. 75, 100) will replace it based upon the value of 'size'.
* @param {string} props.fill
Expand Down Expand Up @@ -48,22 +48,50 @@ export const Template = ({

let idKey = iconName;

// If a descriptor like Right, Left, Down, or Up is present for the Chevron or the
// If icon set was not provided, try determine which icon set contains this icon.
// Note: icon sets can contain the same icon name, with different icons.
if (!['workflow','ui'].includes(setName)){
if (workflowIcons.includes(idKey)) {
setName = "workflow";
} else if (uiIcons.includes(idKey.replace(/\d{2,3}$/, "").replace(/(Right|Left|Down|Up)$/, ""))) {
setName = "ui";
}
}

if (!setName) {
console.warn(
`Icon: Could not determine the icon set for the provided icon name: ${idKey}.`
);
return html``;
}

// If a descriptor like Right, Left, Down, or Up is present for the UI icons Chevron or
// Arrow, use that only for the class and not the icon fetch.
if (
setName == "ui" &&
uiIcons.some((c) => idKey.startsWith(c)) &&
["Right", "Left", "Down", "Up"].some((c) => idKey.includes(c))
) {
idKey = idKey.replace(/(Right|Left|Down|Up)/, "");
}

// If the icon name includes its scale, we want to leave that scale
// If the icon name does not include scale, reformat it to match the provided sizing.
// E.g. with a size of "s", the icon name "ChevronRight" would become "ChevronRight75".
/**
* Fallback UI Icon sizing number.
*
* If the icon name includes its scale, we want to leave that scale. This is preferred,
* as UI icons do not use workflow icon sizing.
*
* If the UI icon name does not include scale, reformat it to match the provided sizing.
* E.g. with a size of "s", the icon name "ChevronRight" would become "ChevronRight75".
*/
if (
setName == "ui" &&
// Exists in the list of available UI icons.
uiIcons.includes(idKey.replace(/\d{2,3}$/, "")) &&
// Does not already have size number at the end.
!idKey.match(/^(?!\d).*\d{2,3}$/) &&
!idKey.endsWith("Gripper")
// Exclude some UI icons that do not (yet) have size numbers.
uiIconSizes[idKey]?.length != 0
) {
let sizeVal;
switch (size) {
Expand All @@ -85,23 +113,9 @@ export const Template = ({

idKey += sizeVal;
iconName += sizeVal;

}

// Determine which icon set contains this icon.
if (workflowIcons.includes(idKey)) {
setName = "workflow";
} else if (uiIcons.includes(idKey.replace(/\d{2,3}$/, ""))) {
setName = "ui";
}

if (!setName) {
console.warn(
`Icon: Could not determine the icon set for the provided icon name: ${idKey}.`
);
return html``;
}

// Fetch SVG file markup, and set optional fill color.
let inlineStyle;
if (fill) inlineStyle = `color: ${fill}`;
let icon;
Expand All @@ -110,11 +124,15 @@ export const Template = ({
icon = fetchIconSVG({ iconName: idKey, setName, ...globals });

if (!icon) {
console.warn(`Icon: ${idKey} not found.`);
console.warn(`Icon: "${idKey}" was not found in the "${setName}" icon set.`);
return html``;
}
}

/**
* Classes to apply to the SVG element. Object as used by the classMap function.
* @type {[name: string]: string | boolean | number}
*/
const classList = {
[rootClass]: true,
[`spectrum-UIIcon-${iconName}`]: !!(setName === "ui"),
Expand All @@ -136,7 +154,7 @@ export const Template = ({
.map(([k]) => k)
.join(" ")}"${
inlineStyle ? ` style="${inlineStyle}"` : ""
} focusable="false" aria-hidden="true" role="img" width="10" $1>`
} focusable="false" aria-hidden="true" role="img" $1>`
)
)}`;
}
Expand Down
Loading

0 comments on commit b63631a

Please sign in to comment.