fix(Button): specify size for enabled/disabled buttons (#2063)

- separate out the size metrics from the internal size styles - regenerate snapshots for comparison - add story for showing disabled buttons at all sizes
chanzuckerberg · Oct 1, 2024 · 39f3af6 · 39f3af6
1 parent 946618e
commit 39f3af6
Show file tree

Hide file tree

Showing 16 changed files with 243 additions and 161 deletions.
diff --git a/src/components/AppNotification/__snapshots__/AppNotification.test.ts.snap b/src/components/AppNotification/__snapshots__/AppNotification.test.ts.snap
@@ -53,7 +53,7 @@ exports[`<AppNotification /> LightColor story renders snapshot 1`] = `
           class="button-group button-group--horizontal-align-left"
         >
           <button
-            class="button button--layout-none button--secondary button--lg button--variant-default"
+            class="button button--layout-none button--secondary button--lg button--size-lg button--variant-default"
             type="button"
           >
             <span
@@ -63,7 +63,7 @@ exports[`<AppNotification /> LightColor story renders snapshot 1`] = `
             </span>
           </button>
           <button
-            class="button button--layout-none button--tertiary button--lg button--variant-default"
+            class="button button--layout-none button--tertiary button--lg button--size-lg button--variant-default"
             type="button"
           >
             <span
@@ -105,7 +105,7 @@ exports[`<AppNotification /> LightWithDismissAndControls story renders snapshot
           class="button-group button-group--horizontal-align-left"
         >
           <button
-            class="button button--layout-none button--secondary button--lg button--variant-default"
+            class="button button--layout-none button--secondary button--lg button--size-lg button--variant-default"
             type="button"
           >
             <span
@@ -115,7 +115,7 @@ exports[`<AppNotification /> LightWithDismissAndControls story renders snapshot
             </span>
           </button>
           <button
-            class="button button--layout-none button--tertiary button--lg button--variant-default"
+            class="button button--layout-none button--tertiary button--lg button--size-lg button--variant-default"
             type="button"
           >
             <span
@@ -129,7 +129,7 @@ exports[`<AppNotification /> LightWithDismissAndControls story renders snapshot
     </section>
     <button
       aria-label="close"
-      class="button button--context-default button--layout-icon-only button--tertiary button--lg button--variant-neutral app-notification__close-btn"
+      class="button button--context-default button--layout-icon-only button--tertiary button--lg button--size-lg button--variant-neutral app-notification__close-btn"
       type="button"
     >
       <span
@@ -181,7 +181,7 @@ exports[`<AppNotification /> WithControls story renders snapshot 1`] = `
           class="button-group button-group--horizontal !flex-row"
         >
           <button
-            class="button button--layout-none button--secondary button--lg button--variant-inverse"
+            class="button button--layout-none button--secondary button--lg button--size-lg button--variant-inverse"
             type="button"
           >
             <span
@@ -191,7 +191,7 @@ exports[`<AppNotification /> WithControls story renders snapshot 1`] = `
             </span>
           </button>
           <button
-            class="button button--layout-none button--tertiary button--lg button--variant-inverse"
+            class="button button--layout-none button--tertiary button--lg button--size-lg button--variant-inverse"
             type="button"
           >
             <span
@@ -233,7 +233,7 @@ exports[`<AppNotification /> WithDismissAndControls story renders snapshot 1`] =
           class="button-group button-group--horizontal !flex-row"
         >
           <button
-            class="button button--layout-none button--secondary button--lg button--variant-inverse"
+            class="button button--layout-none button--secondary button--lg button--size-lg button--variant-inverse"
             type="button"
           >
             <span
@@ -243,7 +243,7 @@ exports[`<AppNotification /> WithDismissAndControls story renders snapshot 1`] =
             </span>
           </button>
           <button
-            class="button button--layout-none button--tertiary button--lg button--variant-inverse"
+            class="button button--layout-none button--tertiary button--lg button--size-lg button--variant-inverse"
             type="button"
           >
             <span
@@ -257,7 +257,7 @@ exports[`<AppNotification /> WithDismissAndControls story renders snapshot 1`] =
     </section>
     <button
       aria-label="close"
-      class="button button--context-default button--layout-icon-only button--tertiary button--lg button--variant-inverse app-notification__close-btn"
+      class="button button--context-default button--layout-icon-only button--tertiary button--lg button--size-lg button--variant-inverse app-notification__close-btn"
       type="button"
     >
       <span

diff --git a/src/components/Button/Button.module.css b/src/components/Button/Button.module.css
@@ -68,27 +68,33 @@
 
   padding: calc(var(--eds-size-1) / 16 * 1rem) calc(var(--eds-size-2-and-half) / 16 * 1rem);
   font: var(--eds-theme-typography-button-lg);
-
-  min-width:calc(var(--eds-size-9) / 16 * 1rem);
-  max-width:calc(var(--eds-size-40) / 16 * 1rem);
-  max-height: calc(var(--eds-size-5) / 16 * 1rem);
 }
 
 .button--md {
   align-items: center;
 
   padding: calc(var(--eds-size-half) / 16 * 1rem) calc(var(--eds-size-2) / 16 * 1rem);
   font: var(--eds-theme-typography-button-md);
-
-  min-width: 3.75rem; /* TODO: missing size token */
-  max-width: calc(var(--eds-size-32) / 16 * 1rem);
-  max-height: calc(var(--eds-size-4) / 16 * 1rem);
 }
 
 .button--sm {
   padding: calc(var(--eds-size-half) / 16 * 1rem) calc(var(--eds-size-1-and-half) / 16 * 1rem);
   font: var(--eds-theme-typography-button-sm);
+}
+
+.button--size-lg {
+  min-width:calc(var(--eds-size-9) / 16 * 1rem);
+  max-width:calc(var(--eds-size-40) / 16 * 1rem);
+  max-height: calc(var(--eds-size-5) / 16 * 1rem);
+}
+
+.button--size-md {
+  min-width: 3.75rem; /* TODO: missing size token */
+  max-width: calc(var(--eds-size-32) / 16 * 1rem);
+  max-height: calc(var(--eds-size-4) / 16 * 1rem);
+}
 
+.button--size-sm {
   min-width: calc(var(--eds-size-6) / 16 * 1rem);
   max-width: calc(var(--eds-size-24) / 16 * 1rem);
   max-height: calc(var(--eds-size-3) / 16 * 1rem);

diff --git a/src/components/Button/Button.stories.tsx b/src/components/Button/Button.stories.tsx
@@ -181,6 +181,21 @@ export const FullWidths: Story = {
   render: Sizes.render,
 };
 
+/**
+ * Buttons can come with full width set, which will expand the button to its maximum width (diferent for each size). Respects Disabled
+ */
+export const DisabledFullWidths: Story = {
+  args: {
+    ...Sizes.args,
+    isFullWidth: true,
+    isDisabled: true,
+  },
+  parameters: {
+    layout: 'padded',
+  },
+  render: Sizes.render,
+};
+
 /**
  * When in the loading state, a button will show a loading indicator in place of the normal button text, maintaining the initial size.
  */

diff --git a/src/components/Button/Button.tsx b/src/components/Button/Button.tsx
@@ -123,6 +123,7 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
       isLoading && styles['button--loading'],
       rank && styles[`button--${rank}`],
       size && styles[`button--${size}`],
+      size && styles[`button--size-${size}`],
       variant && styles[`button--variant-${variant}`],
       !isDisabled && className,
     );
@@ -186,7 +187,15 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
 
     // Wrap the button in a simple SPAN to allow for adding the not-allowed cursor
     return isDisabled ? (
-      <span className={clsx(className, styles['button__disabled'])}>
+      <span
+        className={clsx(
+          className,
+          iconLayout && styles[`button--layout-${iconLayout}`],
+          isFullWidth && styles['button--full-width'],
+          size && styles[`button--size-${size}`],
+          styles['button__disabled'],
+        )}
+      >
         {coreButton}
       </span>
     ) : (