Skip to content

Commit

Permalink
feat(closebutton): fix close button sizing class name (#3108)
Browse files Browse the repository at this point in the history
  • Loading branch information
castastrophe authored Sep 13, 2024
1 parent e36343b commit 7fedb1f
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 9 deletions.
7 changes: 7 additions & 0 deletions .changeset/many-eels-protect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@spectrum-css/closebutton": minor
---

Currently the t-shirt sizing for Close button is using "spectrum-Closebutton" as a prefix which does not align with the use of "spectrum-CloseButton" (capital B) for the root class. This PR adds sizing classes that use the capital B and labels the lowercase B class as deprecated.

Expands Close button test coverage to include the hover and focus states.
16 changes: 10 additions & 6 deletions components/closebutton/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,6 @@
*/

@import "./themes/express.css";

/* closebutton/index.css
*
* .spectrum-Closebutton::after is the Focus ring
*/

@import "@spectrum-css/commons/basebutton.css";

.spectrum-CloseButton {
Expand Down Expand Up @@ -48,27 +42,36 @@
--spectrum-closebutton-animation-duration: var(--spectrum-animation-duration-100);
}

/* @deprecated .spectrum-Closebutton--sizeS */
.spectrum-CloseButton--sizeS,
.spectrum-Closebutton--sizeS {
--spectrum-closebutton-height: var(--spectrum-component-height-75);
--spectrum-closebutton-width: var(--spectrum-closebutton-height);
--spectrum-closebutton-size: var(--spectrum-closebutton-size-300);
--spectrum-closebutton-border-radius: var(--spectrum-closebutton-size-300);
}

/* @deprecated .spectrum-Closebutton--sizeM */
.spectrum-CloseButton,
.spectrum-CloseButton--sizeM,
.spectrum-Closebutton--sizeM {
--spectrum-closebutton-height: var(--spectrum-component-height-100);
--spectrum-closebutton-width: var(--spectrum-closebutton-height);
--spectrum-closebutton-size: var(--spectrum-closebutton-size-400);
--spectrum-closebutton-border-radius: var(--spectrum-closebutton-size-400);
}

/* @deprecated .spectrum-Closebutton--sizeL */
.spectrum-CloseButton--sizeL,
.spectrum-Closebutton--sizeL {
--spectrum-closebutton-height: var(--spectrum-component-height-200);
--spectrum-closebutton-width: var(--spectrum-closebutton-height);
--spectrum-closebutton-size: var(--spectrum-closebutton-size-500);
--spectrum-closebutton-border-radius: var(--spectrum-closebutton-size-500);
}

/* @deprecated .spectrum-Closebutton--sizeXL */
.spectrum-CloseButton--sizeXL,
.spectrum-Closebutton--sizeXL {
--spectrum-closebutton-height: var(--spectrum-component-height-300);
--spectrum-closebutton-width: var(--spectrum-closebutton-height);
Expand Down Expand Up @@ -160,6 +163,7 @@ a.spectrum-CloseButton {
margin-block-start: var(--mod-closebutton-margin-top);
align-self: var(--mod-closebutton-align-self);

/* Represents focus ring */
&::after {
pointer-events: none;
content: "";
Expand Down
4 changes: 4 additions & 0 deletions components/closebutton/metadata/metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
"sourceFile": "index.css",
"selectors": [
".spectrum-CloseButton",
".spectrum-CloseButton--sizeL",
".spectrum-CloseButton--sizeM",
".spectrum-CloseButton--sizeS",
".spectrum-CloseButton--sizeXL",
".spectrum-CloseButton--staticBlack",
".spectrum-CloseButton--staticBlack.is-focused:not(:disabled) .spectrum-CloseButton-UIIcon",
".spectrum-CloseButton--staticBlack.is-keyboardFocused:not(:disabled)",
Expand Down
8 changes: 7 additions & 1 deletion components/closebutton/stories/closebutton.stories.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { disableDefaultModes } from "@spectrum-css/preview/modes";
import { isDisabled, size, staticColor } from "@spectrum-css/preview/types";
import { isDisabled, isFocused, isHovered, isKeyboardFocused, size, staticColor } from "@spectrum-css/preview/types";
import pkgJson from "../package.json";
import { CloseButtonGroup } from "./closebutton.test.js";

Expand All @@ -13,11 +13,17 @@ export default {
size: size(["s", "m", "l", "xl"]),
staticColor,
isDisabled,
isHovered,
isFocused,
isKeyboardFocused,
},
args: {
rootClass: "spectrum-CloseButton",
size: "m",
isDisabled: false,
isHovered: false,
isFocused: false,
isKeyboardFocused: false,
},
parameters: {
actions: {
Expand Down
17 changes: 17 additions & 0 deletions components/closebutton/stories/closebutton.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,26 @@ export const CloseButtonGroup = Variants({
},
],
stateData: [
{
testHeading: "Hovered",
isHovered: true,
},
{
testHeading: "Focused",
isFocused: true,
},
{
testHeading: "Keyboard-focused",
isKeyboardFocused: true,
},
{
testHeading: "Disabled",
isDisabled: true,
},
{
testHeading: "Disabled + hovered",
isDisabled: true,
isHovered: true,
},
]
});
10 changes: 8 additions & 2 deletions components/closebutton/stories/template.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ export const Template = ({
label = "Close",
staticColor,
isDisabled = false,
isHovered = false,
isFocused = false,
isKeyboardFocused = false,
customClasses = [],
id = getRandomId("closebutton"),
onclick,
Expand All @@ -22,8 +25,11 @@ export const Template = ({
class=${classMap({
[rootClass]: true,
[`${rootClass}--size${upperCase(size)}`]: typeof size !== "undefined",
[`${rootClass}--static${capitalize(staticColor)}`]:
typeof staticColor !== "undefined",
[`${rootClass}--static${capitalize(staticColor)}`]: typeof staticColor !== "undefined",
"is-disabled": isDisabled,
"is-hover": isHovered,
"is-focus-visible": isFocused,
"is-keyboardFocused": isKeyboardFocused,
...customClasses.reduce((a, c) => ({ ...a, [c]: true }), {}),
})}
aria-label="close"
Expand Down

0 comments on commit 7fedb1f

Please sign in to comment.