Skip to content

Commit

Permalink
fix(picker): rename is-focused class to is-keyboardFocused (#2377)
Browse files Browse the repository at this point in the history
* fix(picker): rename is-focused to is-keyboardFocused

The .is-focused class was being used as if it was .is-keyboardFocused,
in combination with :focus-visible in selectors. Rather than removing the class,
keep it in its renamed form in order for SWC to still be able to convert it to an
attribute and continue testing it via its Storybook control.

This fixes an existing issue on the docs site, where if you click the
picker twice, the focus indicator ring will appear. This is because
the docs site adding the .is-focused class.

Updated the existing story that was labeled "Focused" that was showing
keyboard focused to use the play function, for Chromatic coverage
ref: https://www.chromatic.com/docs/hoverfocus/
  • Loading branch information
jawinn authored Jan 17, 2024
1 parent cf44e69 commit 60b44bb
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 17 deletions.
10 changes: 5 additions & 5 deletions components/picker/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ governing permissions and limitations under the License.
--highcontrast-picker-background-color: ButtonFace;

&:focus-visible,
&.is-focused {
&.is-keyboardFocused {
--highcontrast-picker-border-color-hover: ButtonText;
}

Expand Down Expand Up @@ -265,7 +265,7 @@ governing permissions and limitations under the License.
}

&:focus-visible,
&.is-focused {
&.is-keyboardFocused {
outline: none;
background-color: var(--highcontrast-picker-background-color, var(--mod-picker-background-color-key-focus, var(--spectrum-picker-background-color-key-focus)));
border-color: var(--highcontrast-picker-border-color-default, var(--mod-picker-border-color-key-focus, var(--spectrum-picker-border-color-key-focus)));
Expand Down Expand Up @@ -309,7 +309,7 @@ governing permissions and limitations under the License.
}

&:focus-visible,
&.is-focused {
&.is-keyboardFocused {
border-color: var(--highcontrast-picker-border-color-default, var(--mod-picker-border-color-error-key-focus, var(--spectrum-picker-border-color-error-key-focus)));
}
}
Expand Down Expand Up @@ -348,7 +348,7 @@ governing permissions and limitations under the License.
color: var(--highcontrast-picker-content-color-default, var(--mod-picker-font-color-default-open, var(--spectrum-picker-font-color-default-open)));
background-color: var(--highcontrast-picker-background-color, var(--mod-picker-background-color-default-open, var(--spectrum-picker-background-color-default-open)));
border-color: var(--highcontrast-picker-border-color-default, var(--mod-picker-border-default-open, var(--spectrum-picker-border-color-default-open)));

&:hover {
color: var(--highcontrast-picker-content-color-default, var(--mod-picker-font-color-hover-open, var(--spectrum-picker-font-color-hover-open)));
background-color: var(--highcontrast-picker-background-color, var(--mod-picker-background-color-hover-open, var(--spectrum-picker-background-color-hover-open)));
Expand Down Expand Up @@ -467,7 +467,7 @@ governing permissions and limitations under the License.
}

&:focus-visible,
&.is-focused {
&.is-keyboardFocused {
background-color: var(--highcontrast-picker-background-color, transparent);

/* Focus indicator changes from a ring to a line underneath. */
Expand Down
13 changes: 6 additions & 7 deletions components/picker/stories/picker.stories.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// Import the component markup template
import { Template } from "./template";

import { Default as MenuStories } from "@spectrum-css/menu/stories/menu.stories.js";
import { Template } from "./template";

export default {
title: "Components/Picker",
Expand Down Expand Up @@ -64,8 +62,8 @@ export default {
},
control: "boolean",
},
isFocused: {
name: "Focused",
isKeyboardFocused: {
name: "Keyboard focused",
type: { name: "boolean" },
table: {
type: { summary: "boolean" },
Expand Down Expand Up @@ -108,9 +106,9 @@ export default {
label: "Country",
placeholder: "Select a country",
isQuiet: false,
isKeyboardFocused: false,
isLoading: false,
isDisabled: false,
isFocused: false,
isInvalid: false,
isOpen: false,
},
Expand Down Expand Up @@ -176,9 +174,10 @@ Invalid.args = {
};

export const Focused = Template.bind({});
Focused.storyName = "Keyboard Focused";
Focused.args = {
helpText: "Please select a country",
isFocused: true,
isKeyboardFocused: true,
content: [
() => MenuStories(MenuStories.args)
],
Expand Down
10 changes: 5 additions & 5 deletions components/picker/stories/template.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const Picker = ({
labelPosition,
placeholder,
isQuiet = false,
isFocused = false,
isKeyboardFocused = false,
isOpen = false,
isInvalid = false,
isLoading = false,
Expand Down Expand Up @@ -52,7 +52,7 @@ export const Picker = ({
[`is-invalid`]: isInvalid,
[`is-open`]: isOpen,
[`is-loading`]: isLoading,
[`is-focused`]: isFocused,
[`is-keyboardFocused`]: isKeyboardFocused,
...customClasses.reduce((a, c) => ({ ...a, [c]: true }), {}),
})}
?disabled=${isDisabled}
Expand Down Expand Up @@ -96,7 +96,7 @@ export const Template = ({
placeholder,
helpText,
isQuiet = false,
isFocused = false,
isKeyboardFocused = false,
isOpen = false,
isInvalid = false,
isLoading = false,
Expand Down Expand Up @@ -151,7 +151,7 @@ export const Template = ({
size,
placeholder,
isQuiet,
isFocused,
isKeyboardFocused,
isOpen,
isInvalid,
isLoading,
Expand All @@ -173,7 +173,7 @@ export const Template = ({
size,
placeholder,
isQuiet,
isFocused,
isKeyboardFocused,
isOpen,
isInvalid,
isLoading,
Expand Down

0 comments on commit 60b44bb

Please sign in to comment.