Skip to content

Commit f73d63e

Browse files
fix(tag): the easier pr fixes
ADJUST FOCUS BEHAVIOR: - remove .is-focused states from CSS to use native pseudo-classes - use keyboard focused global state instead of focused global state in Storybook - change of arg from isFocused to isKeyboardFocused in template and tests - removal of focus event listeners to update args ADJUST EMPHASIZED STYLING - .is-emphasized is now .spectrum-Tag--emphasized in CSS and template - emphasized styles only take effect if also selected - update tests and test headings to better display the emphasized variant - small adjustments to story names on docs page to put selected stories next to each other and show emphasized and selected rather than just emphasized ADJUST READ ONLY STYLING - Remove user-select: none to make tag text selectable per guidelines - prevent selected styles from having effect if read-only or disabled - adjust emphasized & read-only tests to include selected state MISC OTHER FIXES - hasClearButton control has been changed to isRemovable - Update of Truncated story to remove max width and rely on component's max width
1 parent 179c892 commit f73d63e

File tree

5 files changed

+60
-61
lines changed

5 files changed

+60
-61
lines changed

components/tag/dist/metadata.json

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,21 @@
77
".spectrum-Tag .spectrum-Tag-itemIcon",
88
".spectrum-Tag .spectrum-Tag-itemLabel",
99
".spectrum-Tag .spectrum-Thumbnail",
10+
".spectrum-Tag--emphasized.is-selected:not(.is-disabled, .is-readOnly)",
11+
".spectrum-Tag--emphasized.is-selected:not(.is-disabled, .is-readOnly):active",
12+
".spectrum-Tag--emphasized.is-selected:not(.is-disabled, .is-readOnly):focus-visible",
13+
".spectrum-Tag--emphasized.is-selected:not(.is-disabled, .is-readOnly):hover",
1014
".spectrum-Tag--sizeL",
1115
".spectrum-Tag--sizeS",
1216
".spectrum-Tag.is-disabled",
1317
".spectrum-Tag.is-disabled .spectrum-Avatar",
1418
".spectrum-Tag.is-disabled .spectrum-Thumbnail",
15-
".spectrum-Tag.is-emphasized",
16-
".spectrum-Tag.is-emphasized.is-focused:not(.is-disabled, .is-readOnly)",
17-
".spectrum-Tag.is-emphasized:not(.is-disabled, .is-readOnly):active",
18-
".spectrum-Tag.is-emphasized:not(.is-disabled, .is-readOnly):focus-visible",
19-
".spectrum-Tag.is-emphasized:not(.is-disabled, .is-readOnly):hover",
20-
".spectrum-Tag.is-focused:not(.is-disabled, .is-readOnly)",
21-
".spectrum-Tag.is-focused:not(.is-disabled, .is-readOnly):after",
2219
".spectrum-Tag.is-selected",
23-
".spectrum-Tag.is-selected.is-focused",
24-
".spectrum-Tag.is-selected:active",
25-
".spectrum-Tag.is-selected:focus-visible",
26-
".spectrum-Tag.is-selected:hover",
20+
".spectrum-Tag.is-selected:not(.is-disabled, .is-readOnly)",
21+
".spectrum-Tag.is-selected:not(.is-disabled, .is-readOnly):active",
22+
".spectrum-Tag.is-selected:not(.is-disabled, .is-readOnly):focus-visible",
23+
".spectrum-Tag.is-selected:not(.is-disabled, .is-readOnly):hover",
24+
".spectrum-Tag.spectrum-Tag--emphasized",
2725
".spectrum-Tag:lang(ja)",
2826
".spectrum-Tag:lang(ko)",
2927
".spectrum-Tag:lang(zh)",

components/tag/index.css

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,6 @@
191191
vertical-align: bottom;
192192
max-inline-size: calc(var(--mod-tag-height, var(--spectrum-tag-height)) * var(--mod-tag-maximum-width-multiplier, var(--spectrum-tag-maximum-width-multiplier)));
193193
outline: none;
194-
user-select: none;
195194

196195
transition:
197196
border-color var(--mod-tag-animation-duration, var(--spectrum-tag-animation-duration)) ease-in-out,
@@ -271,8 +270,7 @@
271270
transform: perspective(max(var(--spectrum-downstate-height), var(--spectrum-downstate-width) * var(--spectrum-component-size-width-ratio-down))) translateZ(var(--spectrum-component-size-difference-down));
272271
}
273272

274-
&:not(.is-disabled, .is-readOnly):focus-visible,
275-
&:not(.is-disabled, .is-readOnly).is-focused {
273+
&:not(.is-disabled, .is-readOnly):focus-visible {
276274
/* mod/highcontrast specified at top */
277275
border-color: var(--spectrum-tag-border-color-focus);
278276
background-color: var(--spectrum-tag-background-color-focus);
@@ -297,7 +295,7 @@
297295
}
298296

299297
/* selected */
300-
.spectrum-Tag.is-selected {
298+
.spectrum-Tag.is-selected:not(.is-disabled, .is-readOnly) {
301299
/* mod/highcontrast colors specified at top */
302300
border-color: var(--spectrum-tag-border-color-selected);
303301
background-color: var(--spectrum-tag-background-color-selected);
@@ -315,35 +313,33 @@
315313
color: var(--spectrum-tag-content-color-selected);
316314
}
317315

318-
&:focus-visible,
319-
&.is-focused {
316+
&:focus-visible {
320317
border-color: var(--spectrum-tag-border-color-selected);
321318
background-color: var(--spectrum-tag-background-color-selected-focus);
322319
color: var(--spectrum-tag-content-color-selected);
323320
}
324321
}
325322

326323
/* emphasized */
327-
.spectrum-Tag.is-emphasized {
324+
.spectrum-Tag--emphasized.is-selected:not(.is-disabled, .is-readOnly) {
328325
/* mod/highcontrast colors specified at top */
329326
border-color: var(--spectrum-tag-border-color-emphasized);
330327
background-color: var(--spectrum-tag-background-color-emphasized);
331328
color: var(--spectrum-tag-content-color-emphasized);
332329

333-
&:not(.is-disabled, .is-readOnly):hover {
330+
&:hover {
334331
border-color: var(--spectrum-tag-border-color-emphasized);
335332
background-color: var(--spectrum-tag-background-color-emphasized-hover);
336333
color: var(--spectrum-tag-content-color-emphasized);
337334
}
338335

339-
&:not(.is-disabled, .is-readOnly):active {
336+
&:active {
340337
border-color: var(--spectrum-tag-border-color-emphasized);
341338
background-color: var(--spectrum-tag-background-color-emphasized-active);
342339
color: var(--spectrum-tag-content-color-emphasized);
343340
}
344341

345-
&:not(.is-disabled, .is-readOnly):focus-visible,
346-
&:not(.is-disabled, .is-readOnly).is-focused {
342+
&:focus-visible {
347343
border-color: var(--spectrum-tag-border-color-emphasized);
348344
background-color: var(--spectrum-tag-background-color-emphasized-focus);
349345
color: var(--spectrum-tag-content-color-emphasized);
@@ -405,7 +401,7 @@
405401
--highcontrast-tag-content-color-disabled: GrayText;
406402
}
407403

408-
&.is-emphasized {
404+
&.spectrum-Tag--emphasized {
409405
--highcontrast-tag-border-color-emphasized: Highlight;
410406

411407
--highcontrast-tag-background-color-emphasized: ButtonFace;

components/tag/stories/tag.stories.js

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { default as IconStories } from "@spectrum-css/icon/stories/icon.stories.js";
22
import { Sizes, withDownStateDimensionCapture } from "@spectrum-css/preview/decorators";
33
import { disableDefaultModes } from "@spectrum-css/preview/modes";
4-
import { isActive, isDisabled, isEmphasized, isFocused, isHovered, isReadOnly, isSelected, size } from "@spectrum-css/preview/types";
4+
import { isActive, isDisabled, isEmphasized, isHovered, isKeyboardFocused, isReadOnly, isSelected, size } from "@spectrum-css/preview/types";
55
import metadata from "../dist/metadata.json";
66
import packageJson from "../package.json";
77
import { TagGroups } from "./tag.test.js";
@@ -63,10 +63,10 @@ export default {
6363
isDisabled,
6464
isSelected,
6565
isHovered,
66-
isFocused,
66+
isKeyboardFocused,
6767
isActive,
6868
isReadOnly,
69-
hasClearButton: {
69+
isRemovable: {
7070
name: "Removable",
7171
description: "Has a clear button to clear the tag.",
7272
type: { name: "boolean" },
@@ -88,10 +88,10 @@ export default {
8888
isDisabled: false,
8989
isEmphasized: false,
9090
isHovered: false,
91-
isFocused: false,
91+
isKeyboardFocused: false,
9292
isActive: false,
9393
isReadOnly: false,
94-
hasClearButton: false,
94+
isRemovable: false,
9595
},
9696
parameters: {
9797
actions: {
@@ -130,7 +130,9 @@ WithForcedColors.parameters = {
130130
// ********* DOCS ONLY ********* //
131131
/**
132132
* Tags should always include a label to represent search terms, filters, or keywords. Tags also
133-
* have the option to include an icon, avatar, or thumbnail in addition to the label.
133+
* have the option to include an [icon](?path=/docs/components-icon--docs),
134+
* [avatar](?path=/docs/components-avatar--docs), or
135+
* [thumbnail](?path=/docs/components-thumbnail--docs) in addition to the label.
134136
*/
135137
export const Standard = TagsDefaultOptions.bind({});
136138
Standard.args = Default.args;
@@ -142,7 +144,7 @@ Standard.parameters = {
142144

143145

144146
export const Selected = TagsDefaultOptions.bind({});
145-
Selected.storyName = "Default, selected";
147+
Selected.storyName = "Selected default";
146148
Selected.tags = ["!dev"];
147149
Selected.args = {
148150
isSelected: true
@@ -165,22 +167,24 @@ Disabled.parameters = {
165167
};
166168

167169
export const Emphasized = TagsDefaultOptions.bind({});
170+
Emphasized.storyName = "Selected emphasized";
168171
Emphasized.tags = ["!dev"];
169172
Emphasized.args = {
170-
isEmphasized: true
173+
isEmphasized: true,
174+
isSelected: true,
171175
};
172176
Emphasized.parameters = {
173177
chromatic: { disableSnapshot: true },
174178
};
175179

176180
/**
177-
* Tags have the option to be removable or not. Removable tags have a small close ("x") button.
181+
* Tags have the option to be removable or not. Removable tags have a small clear ("x") button.
178182
*/
179183
export const Removable = TagsDefaultOptions.bind({});
180184
Removable.storyName = "Default, removable";
181185
Removable.tags = ["!dev"];
182186
Removable.args = {
183-
hasClearButton: true,
187+
isRemovable: true,
184188
};
185189
Removable.parameters = {
186190
chromatic: { disableSnapshot: true },
@@ -191,7 +195,7 @@ Removable.parameters = {
191195
* Read-only tags cannot be interacted with or changed.
192196
*/
193197
export const ReadOnly = TagsDefaultOptions.bind({});
194-
ReadOnly.storyName = "Read only";
198+
ReadOnly.storyName = "Read-only";
195199
ReadOnly.tags = ["!dev"];
196200
ReadOnly.args = {
197201
isReadOnly: true,
@@ -213,16 +217,22 @@ Sizing.parameters = {
213217

214218

215219
/**
216-
* When the tag text is too long for the available horizontal space, it truncates. The full text should be revealed with a tooltip on hover.
220+
* When the tag text is too long for the available horizontal space, it truncates. The full text
221+
* should be revealed with a tooltip on hover. Tags have a maximum width that differs depending on
222+
* the size of the tag.
217223
* */
218224

219-
export const TextOverflow = TagGroups.bind({});
225+
export const TextOverflow = (args, context) => Sizes({
226+
Template: TagGroups,
227+
withHeading: false,
228+
withBorder: false,
229+
...args,
230+
}, context);
220231
TextOverflow.tags = ["!dev"];
221232
TextOverflow.args = {
222233
hasIcon: true,
223234
iconName: "CheckmarkCircle",
224235
label: "An example of text overflow behavior. When the button text is too long for the horizontal space available, it will truncate and stay on one line.",
225-
customStyles: { "max-inline-size": "200px" }
226236
};
227237

228238
TextOverflow.parameters = {

components/tag/stories/tag.test.js

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ export const TagGroups = Variants({
77
sizeDirection: "row",
88
testData: [
99
{
10-
testHeading: "Default",
10+
testHeading: "Default, not selected",
1111
},
1212
{
13-
testHeading: "Emphasized",
13+
testHeading: "Emphasized, not selected",
1414
isEmphasized: true,
1515
},
1616
{
@@ -25,17 +25,18 @@ export const TagGroups = Variants({
2525
// show removable tags for these variants so hover and focus can be captured
2626
{
2727
testHeading: "Default, removable",
28-
hasClearButton: true,
28+
isRemovable: true,
2929
},
3030
{
31-
testHeading: "Emphasized, removable",
31+
testHeading: "Emphasized & selected, removable",
3232
isEmphasized: true,
33-
hasClearButton: true,
33+
isSelected: true,
34+
isRemovable: true,
3435
},
3536
{
3637
testHeading: "Selected, removable",
3738
isSelected: true,
38-
hasClearButton: true,
39+
isRemovable: true,
3940
},
4041
{
4142
testHeading: "Disabled with states",
@@ -61,9 +62,10 @@ export const TagGroups = Variants({
6162
isReadOnly: true,
6263
},
6364
{
64-
testHeading: "Emphasized, read-only",
65+
testHeading: "Emphasized & selected, read-only",
6566
isReadOnly: true,
6667
isEmphasized: true,
68+
isSelected: true,
6769
},
6870
// truncated, which many states below ignore
6971
{
@@ -75,7 +77,7 @@ export const TagGroups = Variants({
7577
// show removable as a state for test headings that don't already test it
7678
{
7779
testHeading: "Removable",
78-
hasClearButton: true,
80+
isRemovable: true,
7981
include: ["Default, with icon" , "Default, with avatar", "Emphasized, with thumbnail", "Truncated"],
8082
},
8183
{
@@ -84,8 +86,8 @@ export const TagGroups = Variants({
8486
ignore: ["Truncated"],
8587
},
8688
{
87-
testHeading: "Focused",
88-
isFocused: true,
89+
testHeading: "Keyboard focused",
90+
isKeyboardFocused: true,
8991
ignore: ["Truncated"],
9092
},
9193
{

components/tag/stories/template.js

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ export const Template = ({
2222
isEmphasized = false,
2323
isDisabled = false,
2424
isHovered = false,
25-
isFocused = false,
25+
isKeyboardFocused = false,
2626
isActive = false,
2727
isReadOnly = false,
28-
hasClearButton = false,
28+
isRemovable = false,
2929
id = getRandomId("tag"),
3030
customClasses = [],
3131
customStyles = {},
@@ -38,11 +38,11 @@ export const Template = ({
3838
[rootClass]: true,
3939
[`${rootClass}--size${size?.toUpperCase()}`]:
4040
typeof size !== "undefined",
41-
"is-emphasized": isEmphasized,
41+
[`${rootClass}--emphasized`]: isEmphasized,
4242
"is-disabled": isDisabled,
4343
"is-selected": isSelected,
4444
"is-hover": isHovered,
45-
"is-focus-visible": isFocused,
45+
"is-focus-visible": isKeyboardFocused,
4646
"is-active": isActive,
4747
"is-readOnly": isReadOnly,
4848
...customClasses.reduce((a, c) => ({ ...a, [c]: true }), {}),
@@ -54,13 +54,6 @@ export const Template = ({
5454
if (isReadOnly || isDisabled) return;
5555
updateArgs({ isSelected: !isSelected });
5656
}}
57-
@focusin=${function() {
58-
if (isReadOnly || isDisabled) return;
59-
updateArgs({ isFocused: true });
60-
}}
61-
@focusout=${function() {
62-
updateArgs({ isFocused: false });
63-
}}
6457
>
6558
${when(avatarUrl, () =>
6659
Avatar({
@@ -81,7 +74,7 @@ export const Template = ({
8174
}, context)
8275
)}
8376
<span class="${rootClass}-itemLabel">${label}</span>
84-
${when(hasClearButton, () =>
77+
${when(isRemovable, () =>
8578
ClearButton({
8679
isDisabled,
8780
customClasses: [`${rootClass}-clearButton`],
@@ -108,9 +101,9 @@ export const TagsDefaultOptions = ({
108101
},
109102
content: html`
110103
${Template(args, context)}
111-
${args.hasClearButton ? "" : Template({
104+
${args.isRemovable ? "" : Template({
112105
...args,
113-
hasClearButton: true,
106+
isRemovable: true,
114107
}, context)}
115108
${Template({
116109
...args,

0 commit comments

Comments
 (0)