Fix underline

Khan · Dec 19, 2024 · bcb6ced · bcb6ced
1 parent ff165fa
commit bcb6ced
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 28 deletions.
diff --git a/__docs__/wonder-blocks-button/button.stories.tsx b/__docs__/wonder-blocks-button/button.stories.tsx
@@ -135,24 +135,6 @@ export const Tertiary: StoryComponentType = {
     },
 };
 
-export const Focus: StoryComponentType = {
-    args: {
-        onClick: () => {},
-        kind: "primary",
-        children: "Hello, world!",
-    },
-    play: async ({canvasElement}) => {
-        const canvas = within(canvasElement);
-
-        // Get HTML elements
-        const button = canvas.getByRole("button");
-
-        // Focus style
-        await userEvent.tab(button);
-        await expect(button).toHaveFocus();
-    },
-};
-
 export const styles: StyleDeclaration = StyleSheet.create({
     row: {
         flexDirection: "row",

diff --git a/packages/wonder-blocks-button/src/components/button-core.tsx b/packages/wonder-blocks-button/src/components/button-core.tsx
@@ -110,6 +110,7 @@ const ButtonCore: React.ForwardRefExoticComponent<
             <Label
                 style={[
                     sharedStyles.text,
+                    size === "small" && sharedStyles.smallText,
                     size === "large" && sharedStyles.largeText,
                     labelStyle,
                     spinner && sharedStyles.hiddenText,
@@ -276,14 +277,16 @@ const themedSharedStyles: ThemedStylesFn<ButtonThemeContract> = (theme) => ({
         alignItems: "center",
         fontWeight: theme.font.weight.default,
         whiteSpace: "nowrap",
-        // TODO(juan): Figure out how to use text-underline with this for
-        // tertiary variant
         overflow: "hidden",
-        // contain: "paint",
+        // To account for the underline-offset in tertiary buttons
+        lineHeight: `${theme.font.lineHeight.default}px`,
         textOverflow: "ellipsis",
         display: "inline-block", // allows the button text to truncate
         pointerEvents: "none", // fix Safari bug where the browser was eating mouse events
     },
+    smallText: {
+        lineHeight: `${theme.font.lineHeight.small}px`,
+    },
     largeText: {
         fontSize: theme.font.size.large,
         lineHeight: `${theme.font.lineHeight.large}px`,
@@ -383,14 +386,15 @@ export const _generateStyles = (
                 color: light ? color : theme.color.text.inverse,
                 paddingLeft: padding,
                 paddingRight: padding,
-                // TODO(juan): Change this when we get final designs for hover.
+                // TODO(WB-1844): Change this when we get final designs for
+                // hover.
                 [":hover:not([aria-disabled=true])" as any]: focusStyling,
                 [":focus-visible:not([aria-disabled=true])" as any]:
                     focusStyling,
                 [":active:not([aria-disabled=true])" as any]:
                     activePressedStyling,
             },
-            // focused: focusStyling,
+            focused: focusStyling,
             pressed: activePressedStyling,
             disabled: {
                 background: light
@@ -448,6 +452,8 @@ export const _generateStyles = (
                 borderWidth: theme.border.width.secondary,
                 paddingLeft: padding,
                 paddingRight: padding,
+                // TODO(WB-1844): Change this when we get final designs for
+                // hover.
                 [":hover:not([aria-disabled=true])" as any]: focusStyling,
                 [":focus-visible:not([aria-disabled=true])" as any]:
                     focusStyling,
@@ -480,6 +486,7 @@ export const _generateStyles = (
         };
         const activePressedStyling = {
             color: light ? fadedColor : activeColor,
+            textDecoration: `underline ${theme.size.underline.active}px`,
         };
 
         newStyles = {
@@ -489,8 +496,8 @@ export const _generateStyles = (
                 paddingLeft: 0,
                 paddingRight: 0,
                 [":hover:not([aria-disabled=true])" as any]: {
-                    textUnderlineOffset: 3,
-                    textDecoration: "underline",
+                    textUnderlineOffset: theme.font.offset.default,
+                    textDecoration: `underline ${theme.size.underline.hover}px`,
                 },
                 [":focus-visible:not([aria-disabled=true])" as any]:
                     focusStyling,

diff --git a/packages/wonder-blocks-button/src/themes/default.ts b/packages/wonder-blocks-button/src/themes/default.ts
@@ -1,5 +1,10 @@
 import * as tokens from "@khanacademy/wonder-blocks-tokens";
 
+// The underline-offset is the distance between the text baseline and the
+// bottom of the underline. This is necessary to prevent the underline from
+// breaking with descenders.
+const textUnderlineOffset = tokens.spacing.xxxSmall_4;
+
 const theme = {
     color: {
         bg: {
@@ -106,8 +111,6 @@ const theme = {
         radius: {
             // default
             default: tokens.border.radius.medium_4,
-            // tertiary
-            // tertiary: tokens.border.radius.xSmall_2,
             // small button
             small: tokens.border.radius.medium_4,
             // large button
@@ -126,6 +129,10 @@ const theme = {
             medium: 40,
             large: 56,
         },
+        underline: {
+            hover: tokens.spacing.xxxxSmall_2,
+            active: 1,
+        },
     },
     margin: {
         icon: {
@@ -145,11 +152,16 @@ const theme = {
             large: 18,
         },
         lineHeight: {
-            large: tokens.font.lineHeight.medium,
+            small: tokens.font.lineHeight.small + textUnderlineOffset,
+            default: tokens.font.lineHeight.medium + textUnderlineOffset,
+            large: tokens.font.lineHeight.medium + 2 + textUnderlineOffset,
         },
         weight: {
             default: tokens.font.weight.bold,
         },
+        offset: {
+            default: textUnderlineOffset,
+        },
     },
 };