Skip to content

Commit

Permalink
docs: storybook - fix duplicate ids for checkbox and switch (#2230)
Browse files Browse the repository at this point in the history
* docs(checkbox): fix duplicate ids appearing in storybook

The same value was being used for the ID attribute on the checkbox
label and checkbox itself. This update makes sure they are both given
different IDs based on the passed in ID, by adding a suffix to the ID
used on the checkbox input.

This also fixes the markup previously showing an empty ID attribute on
the checkbox when the ID was not set.

* docs(switch): fix duplicate ids appearing in storybook

The switch was using a hardcoded ID for the input and its label, so
showing multiple switches resulted in duplicate IDs. This now sets
unique IDs based on the passed in ID, along with a fallback.

* docs(menu): fix for duplicate ids in some stories

Fix console warning "Duplicate form field id in the same form" for
the Menu stories that use checkboxes and switches.

* docs(checkbox): set id for default story

Clear up console warning "A form field element should have an id or
name attribute" by adding an ID to the default Checkbox story.

* docs(checkbox): update input id value

Update created unique ID for checkbox input so that it won't result in an ID like default-checkbox-checkbox

---------

Co-authored-by: [ Cassondra ] <castastrophe@users.noreply.github.com>
  • Loading branch information
jawinn and castastrophe authored Oct 30, 2023
1 parent 7801316 commit 17b3374
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 7 deletions.
4 changes: 3 additions & 1 deletion components/checkbox/stories/checkbox.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,6 @@ export default {
};

export const Default = Template.bind({});
Default.args = {};
Default.args = {
id: "default-checkbox",
};
2 changes: 1 addition & 1 deletion components/checkbox/stories/template.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export const Template = ({
if (isDisabled) return;
updateArgs({ isChecked: !isChecked });
}}
id=${id}
id=${ifDefined(id ? `${id}-input` : undefined)}
/>
<span class="${rootClass}-box">
${Icon({
Expand Down
10 changes: 7 additions & 3 deletions components/menu/stories/template.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { html, css } from "lit";
import { html } from "lit";
import { classMap } from "lit/directives/class-map.js";
import { styleMap } from "lit/directives/style-map.js";
import { ifDefined } from "lit/directives/if-defined.js";
import { styleMap } from "lit/directives/style-map.js";

import { Template as Checkbox } from "@spectrum-css/checkbox/stories/template.js";
import { Template as Divider } from "@spectrum-css/divider/stories/template.js";
import { Template as Icon } from "@spectrum-css/icon/stories/template.js";
import { Template as Checkbox } from "@spectrum-css/checkbox/stories/template.js";
import { Template as Switch } from "@spectrum-css/switch/stories/template.js";

import "../index.css";
Expand All @@ -28,6 +28,7 @@ export const MenuItem = ({
items = [],
size,
id,
idx = 0,
hasActions,
selectionMode,
value,
Expand Down Expand Up @@ -103,6 +104,7 @@ export const MenuItem = ({
size,
isEmphasized: true,
label: label,
id: `checkbox-${idx}`,
customClasses: [
`${rootClass}Checkbox`,
],
Expand All @@ -128,6 +130,7 @@ export const MenuItem = ({
...globals,
size,
label: null,
id: `switch-${idx}`,
customClasses: [
`${rootClass}Switch`,
],
Expand Down Expand Up @@ -220,6 +223,7 @@ export const Template = ({
return MenuItem({
...globals,
...i,
idx,
rootClass: `${rootClass}-item`,
role: subrole,
size,
Expand Down
13 changes: 11 additions & 2 deletions components/switch/stories/template.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ export const Template = ({
console.warn(e);
}

// ID attribute value for the input element.
const inputId = id ? `${id}-input` : 'switch-onoff-0';

return html`
<div
class=${classMap({
Expand All @@ -36,10 +39,16 @@ export const Template = ({
})}
id=${ifDefined(id)}
>
<input type="checkbox" class="${rootClass}-input" id="switch-onoff-0" ?disabled=${isDisabled} ?checked=${isChecked}/>
<input
type="checkbox"
class="${rootClass}-input"
id=${inputId}
?disabled=${isDisabled}
?checked=${isChecked}
/>
<span class="${rootClass}-switch"></span>
${label
? html`<label class="${rootClass}-label" for="switch-onoff-0"
? html`<label class="${rootClass}-label" for=${inputId}
>${label}</label
>`
: ""}
Expand Down

0 comments on commit 17b3374

Please sign in to comment.