fix(Button): mark disable usage as invalid (#1977)

- update stories to document
chanzuckerberg · Jun 6, 2024 · 99a70d3 · 99a70d3
1 parent 8389f35
commit 99a70d3
Show file tree

Hide file tree

Showing 4 changed files with 108 additions and 42 deletions.
diff --git a/src/components/Button/Button-v2.module.css b/src/components/Button/Button-v2.module.css
@@ -259,57 +259,65 @@
 /**
  * Disabled
  */
+.button--disabled {
+ &.button--primary.button--variant-default,
+ &.button--primary.button--variant-critical,
+ &.button--primary.button--variant-neutral {
+ color: var(--eds-theme-color-text-utility-disabled-primary);
+ border-color: var(--eds-theme-color-background-utility-disabled-medium-emphasis);
+ background-color: var(--eds-theme-color-background-utility-disabled-medium-emphasis);
+
+ pointer-events: none;
+ }
 
-.button--primary.button--variant-default.button--disabled,
-.button--primary.button--variant-critical.button--disabled,
-.button--primary.button--variant-neutral.button--disabled {
- color: var(--eds-theme-color-text-utility-disabled-primary);
- border-color: var(--eds-theme-color-background-utility-disabled-medium-emphasis);
- background-color: var(--eds-theme-color-background-utility-disabled-medium-emphasis);
+ &.button--primary.button--variant-inverse {
+ color: var(--eds-theme-color-text-utility-inverse-disabled);
+ /* Using transparent for border color to avoid blending opacity of border and background */
+ border-color: transparent;
+ background-color: var(--eds-theme-color-background-utility-inverse-disabled);
 
- pointer-events: none;
-}
+  pointer-events: none;
+ }
 
-.button--primary.button--variant-inverse.button--disabled {
- color: var(--eds-theme-color-text-utility-inverse-disabled);
- /* Using transparent for border color to avoid blending opacity of border and background */
- border-color: transparent;
- background-color: var(--eds-theme-color-background-utility-inverse-disabled);
+ &.button--secondary.button--variant-default,
+ &.button--secondary.button--variant-critical,
+ &.button--secondary.button--variant-neutral {
+ color: var(--eds-theme-color-text-utility-disabled-primary);
+ border-color: var(--eds-theme-color-border-disabled);
+ background-color: var(--eds-theme-color-background-utility-default-no-emphasis);
 
- pointer-events: none;
-}
+  pointer-events: none;
+ }
 
-.button--secondary.button--variant-default.button--disabled,
-.button--secondary.button--variant-critical.button--disabled,
-.button--secondary.button--variant-neutral.button--disabled {
- color: var(--eds-theme-color-text-utility-disabled-primary);
- border-color: var(--eds-theme-color-border-disabled);
- background-color: var(--eds-theme-color-background-utility-default-no-emphasis);
+ &.button--secondary.button--variant-inverse {
+ color: var(--eds-theme-color-text-utility-inverse-disabled);
+ border-color: var(--eds-theme-color-border-utility-inverse-disabled);
 
- pointer-events: none;
-}
+  pointer-events: none;
+ }
 
-.button--secondary.button--variant-inverse.button--disabled {
- color: var(--eds-theme-color-text-utility-inverse-disabled);
- border-color: var(--eds-theme-color-border-utility-inverse-disabled);
+ &.button--tertiary.button--variant-default,
+ &.button--tertiary.button--variant-critical,
+ &.button--tertiary.button--variant-neutral {
+ color: var(--eds-theme-color-text-utility-disabled-primary);
+ background-color: var(--eds-theme-color-background-utility-default-no-emphasis);
 
- pointer-events: none;
-}
+  pointer-events: none;
+ }
 
-.button--tertiary.button--variant-default.button--disabled,
-.button--tertiary.button--variant-critical.button--disabled,
-.button--tertiary.button--variant-neutral.button--disabled {
- color: var(--eds-theme-color-text-utility-disabled-primary);
- background-color: var(--eds-theme-color-background-utility-default-no-emphasis);
+ &.button--tertiary.button--variant-inverse {
+ color: var(--eds-theme-color-text-utility-disabled-primary);
+ border-color: transparent;
 
- pointer-events: none;
+ pointer-events: none;
+ }
 }
 
-.button--tertiary.button--variant-inverse.button--disabled {
-  color: var(--eds-theme-color-text-utility-disabled-primary);
-  border-color: transparent;
-
- pointer-events: none;
+/**
+ * Flag usages of just :disabled as erroneous
+ */
+.button:disabled:not(.button--disabled) {
+ outline: 2px dashed var(--eds-theme-color-border-utility-critical);
 }
 
 /**

diff --git a/src/components/Button/Button-v2.stories.tsx b/src/components/Button/Button-v2.stories.tsx
@@ -36,7 +36,7 @@ type Args = React.ComponentProps<typeof Button>;
 
 export const Default: StoryObj<Args> = {
  args: {
- children: 'Default',
+ children: 'Button',
  },
 };
 
@@ -65,7 +65,7 @@ export const DefaultRanks: StoryObj<Args> = {
 };
 
 /**
- * Buttons can be disabled for each rank
+ * Buttons can be disabled for each rank using `isDisabled`
  */
 export const Disabled: StoryObj<Args> = {
  args: {
@@ -75,6 +75,18 @@ export const Disabled: StoryObj<Args> = {
  render: DefaultRanks.render,
 };
 
+/**
+ * Since `isDisabled` will set the form's proper disabled state, don't use just `disabled`. This will show a visual error.
+ */
+export const JustDisabledProp: StoryObj<Args> = {
+ args: {
+ ...DefaultRanks.args,
+ disabled: true,
+ },
+ render: DefaultRanks.render,
+ decorators: [(Story) => <div className="p-1">{Story()}</div>],
+};
+
 /**
  * Tertiary buttons can have an additional level of emphasis when stood by themselves. Use this case sparingly.
  */

diff --git a/src/components/Button/Button-v2.tsx b/src/components/Button/Button-v2.tsx
@@ -68,6 +68,8 @@ export type ButtonV2Props<ExtendedElement = unknown> =
 
  /**
  * Whether `Button` is set to disabled state (disables interaction and updates appearance).
+ *
+ * Note: this will set the internal field to `disabled` as well. Prefer this to just setting `disabled`.
  */
  isDisabled?: boolean;
 

diff --git a/src/components/Button/__snapshots__/Button-v2.test.tsx.snap b/src/components/Button/__snapshots__/Button-v2.test.tsx.snap
@@ -45,7 +45,7 @@ exports[`<Button /> (v2) Default story renders snapshot 1`] = `
  <span
  class="button__text"
  >
- Default
+ Button
  </span>
 </button>
 `;
@@ -328,6 +328,50 @@ exports[`<Button /> (v2) InverseRanks story renders snapshot 1`] = `
 </div>
 `;
 
+exports[`<Button /> (v2) JustDisabledProp story renders snapshot 1`] = `
+<div
+ class="p-1"
+>
+ <div
+ class="flex gap-1"
+ >
+ <button
+ class="button button--layout-none button--primary button--lg button--variant-default"
+ disabled=""
+ type="button"
+ >
+ <span
+ class="button__text"
+ >
+ Primary
+ </span>
+ </button>
+ <button
+ class="button button--layout-none button--secondary button--lg button--variant-default"
+ disabled=""
+ type="button"
+ >
+ <span
+ class="button__text"
+ >
+ Secondary
+ </span>
+ </button>
+ <button
+ class="button button--layout-none button--tertiary button--lg button--variant-default"
+ disabled=""
+ type="button"
+ >
+ <span
+ class="button__text"
+ >
+ Tertiary
+ </span>
+ </button>
+ </div>
+</div>
+`;
+
 exports[`<Button /> (v2) LoadingStates story renders snapshot 1`] = `
 <div
  class="flex items-center gap-1"