Updated TextField & TextArea styling (for consistency and addresses c…

…ropped outline) (#2311) ## Summary: Work was initially done in #2302 to update the TextField state styling so it was consistent with other components like SingleSelect, MultiSelect, and TextArea. The new styling had some issues in webapp where the focus outline would get cropped off if an ancestor element had `overflow: hidden`. The new styles were reverted in #2309 to unblock other WB changes at the time. This PR re-applies those styles and addresses the focus outline issue in both the TextField and TextArea components. The cropped focus outline issue is resolved by making sure the outlines are within the component. This makes it so the outline is never cropped off if a parent or ancestor element has `overflow: hidden`. The changes that address the outline issue are (see 51d9e4f): - using a negative offset for outlines so that they appear within the component (instead of outside) - simplifying outline styling by removing the box-shadow styling. Only border + outline are used for styling now Other alternatives considered: - Keeping the outline outside and adding additional padding + negative margin so there's enough space for the outline - I think adding spacing around our components isn't ideal because spacing is often added to the components and would override the built in spacing - Adding the outline styles to a pseudo element (`:after`) - doesn't work with input elements - Continue using box-shadow - box-shadow styling also gets cropped out. Could use `inset` option though. I think using outline simplifies things though! ## Test Plan - Confirm that the states for TextField and TextArea are as expected. Note: I added `overflow: hidden` to the component variant stories to simulate the issue (see before & after screenshots) - TextField `?path=/docs/packages-form-textfield-all-variants--docs` - TextArea `?path=/docs/packages-form-textarea-all-variants--docs` | Before | After | | --- | --- | | <img width="1840" alt="Screenshot 2024-08-29 at 8 58 51 AM" src="https://github.com/user-attachments/assets/a9e30b3b-39f2-4807-be14-0554b862542e"> | <img width="1840" alt="Screenshot 2024-08-29 at 8 59 06 AM" src="https://github.com/user-attachments/assets/0d812895-5217-496a-ba72-8c312a246168"> | | <img width="1840" alt="Screenshot 2024-08-29 at 8 59 37 AM" src="https://github.com/user-attachments/assets/0f761996-ddaf-44be-80d7-ac14b28a89a4"> | <img width="1840" alt="Screenshot 2024-08-29 at 8 59 14 AM" src="https://github.com/user-attachments/assets/83070398-b67a-47e3-9ba4-0886dd097956"> | - Confirmed in webapp that focus outline is not cropped off in TextArea and TextField components Issue: WB-1746 Author: beaesguerra Reviewers: jandrade Required Reviewers: Approved By: jandrade Checks: ✅ Lint (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 2/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 1/2), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 2/2), ✅ Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Lint (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), 🚫 Chromatic - Get results on regular PRs, ⏭️ Publish npm snapshot, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ✅ Lint (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 2/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 1/2), 🚫 Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ⏭️ Publish npm snapshot, ✅ gerald, ⌛ undefined Pull Request URL: #2311
Khan · Sep 16, 2024 · 2dfd5eb · 2dfd5eb
1 parent f451591
commit 2dfd5eb
Show file tree

Hide file tree

Showing 6 changed files with 278 additions and 85 deletions.
diff --git a/.changeset/two-toys-raise.md b/.changeset/two-toys-raise.md
@@ -0,0 +1,6 @@
+---
+"@khanacademy/wonder-blocks-form": patch
+---
+
+- Update `TextField` state styling so that it is consistent with other components like `TextArea`, `SingleSelect`, `MultiSelect` (especially the focus styling). The styling also now uses CSS pseudo-classes for easier testing in Chromatic and debugging in browsers.
+- `TextField` and `TextArea` state styling has also been updated so that any outline styles outside of the component are now applied within the component to prevent cropped focus outlines in places where an ancestor element has `overflow: hidden`.
diff --git a/__docs__/wonder-blocks-form/text-area-variants.stories.tsx b/__docs__/wonder-blocks-form/text-area-variants.stories.tsx
@@ -164,5 +164,6 @@ const styles = StyleSheet.create({
     },
     scenario: {
         gap: spacing.small_12,
+        overflow: "hidden",
     },
 });
diff --git a/__docs__/wonder-blocks-form/text-field-variants.stories.tsx b/__docs__/wonder-blocks-form/text-field-variants.stories.tsx
@@ -0,0 +1,169 @@
+import * as React from "react";
+import {StyleSheet} from "aphrodite";
+import type {Meta, StoryObj} from "@storybook/react";
+
+import {View} from "@khanacademy/wonder-blocks-core";
+import {color, spacing} from "@khanacademy/wonder-blocks-tokens";
+import {LabelLarge, LabelMedium} from "@khanacademy/wonder-blocks-typography";
+import {TextField} from "@khanacademy/wonder-blocks-form";
+
+/**
+ * The following stories are used to generate the pseudo states for the
+ * TextField component. This is only used for visual testing in Chromatic.
+ *
+ * Note: Error state is not shown on initial render if the TextField value is empty.
+ */
+export default {
+    title: "Packages / Form / TextField / All Variants",
+    parameters: {
+        docs: {
+            autodocs: false,
+        },
+    },
+} as Meta;
+
+type StoryComponentType = StoryObj<typeof TextField>;
+
+const longText =
+    "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.";
+const longTextWithNoWordBreak =
+    "Loremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididuntutlaboreetdoloremagnaaliqua";
+
+const states = [
+    {
+        label: "Default",
+        props: {},
+    },
+    {
+        label: "Disabled",
+        props: {disabled: true},
+    },
+    {
+        label: "Error",
+        props: {validate: () => "Error"},
+    },
+];
+const States = (props: {
+    light: boolean;
+    label: string;
+    value?: string;
+    placeholder?: string;
+}) => {
+    return (
+        <View
+            style={[props.light && styles.darkDefault, styles.statesContainer]}
+        >
+            <LabelLarge style={props.light && {color: color.white}}>
+                {props.label}
+            </LabelLarge>
+            <View style={[styles.scenarios]}>
+                {states.map((scenario) => {
+                    return (
+                        <View style={styles.scenario} key={scenario.label}>
+                            <LabelMedium
+                                style={props.light && {color: color.white}}
+                            >
+                                {scenario.label}
+                            </LabelMedium>
+                            <TextField
+                                value=""
+                                onChange={() => {}}
+                                {...props}
+                                {...scenario.props}
+                            />
+                        </View>
+                    );
+                })}
+            </View>
+        </View>
+    );
+};
+
+const AllVariants = () => (
+    <View>
+        {[false, true].map((light) => {
+            return (
+                <React.Fragment key={`light-${light}`}>
+                    <States light={light} label="Default" />
+                    <States light={light} label="With Value" value="Text" />
+                    <States
+                        light={light}
+                        label="With Value (long)"
+                        value={longText}
+                    />
+                    <States
+                        light={light}
+                        label="With Value (long, no word breaks)"
+                        value={longTextWithNoWordBreak}
+                    />
+                    <States
+                        light={light}
+                        label="With Placeholder"
+                        placeholder="Placeholder text"
+                    />
+                    <States
+                        light={light}
+                        label="With Placeholder (long)"
+                        placeholder={longText}
+                    />
+                    <States
+                        light={light}
+                        label="With Placeholder (long, no word breaks)"
+                        placeholder={longTextWithNoWordBreak}
+                    />
+                </React.Fragment>
+            );
+        })}
+    </View>
+);
+
+export const Default: StoryComponentType = {
+    render: AllVariants,
+};
+
+/**
+ * There are currently no hover styles.
+ */
+export const Hover: StoryComponentType = {
+    render: AllVariants,
+    parameters: {pseudo: {hover: true}},
+};
+
+export const Focus: StoryComponentType = {
+    render: AllVariants,
+    parameters: {pseudo: {focusVisible: true}},
+};
+
+export const HoverFocus: StoryComponentType = {
+    name: "Hover + Focus",
+    render: AllVariants,
+    parameters: {pseudo: {hover: true, focusVisible: true}},
+};
+
+/**
+ * There are currently no active styles.
+ */
+export const Active: StoryComponentType = {
+    render: AllVariants,
+    parameters: {pseudo: {active: true}},
+};
+
+const styles = StyleSheet.create({
+    darkDefault: {
+        backgroundColor: color.darkBlue,
+    },
+    statesContainer: {
+        padding: spacing.medium_16,
+    },
+    scenarios: {
+        display: "flex",
+        flexDirection: "row",
+        alignItems: "center",
+        gap: spacing.xxxLarge_64,
+        flexWrap: "wrap",
+    },
+    scenario: {
+        gap: spacing.small_12,
+        overflow: "hidden",
+    },
+});
diff --git a/packages/wonder-blocks-form/src/components/__tests__/labeled-text-field.test.tsx b/packages/wonder-blocks-form/src/components/__tests__/labeled-text-field.test.tsx
@@ -3,7 +3,6 @@ import {render, screen, fireEvent} from "@testing-library/react";
 import {userEvent} from "@testing-library/user-event";
 
 import {StyleSheet} from "aphrodite";
-import {color} from "@khanacademy/wonder-blocks-tokens";
 import LabeledTextField from "../labeled-text-field";
 
 describe("LabeledTextField", () => {
@@ -382,28 +381,6 @@ describe("LabeledTextField", () => {
         expect(input).toBeInTheDocument();
     });
 
-    it("light prop is passed to textfield", async () => {
-        // Arrange
-
-        // Act
-        render(
-            <LabeledTextField
-                label="Label"
-                value=""
-                onChange={() => {}}
-                light={true}
-            />,
-        );
-
-        const textField = await screen.findByRole("textbox");
-        textField.focus();
-
-        // Assert
-        expect(textField).toHaveStyle({
-            boxShadow: `0px 0px 0px 1px ${color.blue}, 0px 0px 0px 2px ${color.white}`,
-        });
-    });
-
     it("style prop is passed to fieldheading", async () => {
         // Arrange
         const styles = StyleSheet.create({

diff --git a/packages/wonder-blocks-form/src/components/text-area.tsx b/packages/wonder-blocks-form/src/components/text-area.tsx
@@ -1,5 +1,5 @@
 import * as React from "react";
-import {CSSProperties, Falsy, StyleSheet} from "aphrodite";
+import {StyleSheet} from "aphrodite";
 
 import {
     AriaProps,
@@ -256,7 +256,7 @@ const TextArea = React.forwardRef<HTMLTextAreaElement, TextAreaProps>(
             }
         });
 
-        const getStyles = (): (CSSProperties | Falsy)[] => {
+        const getStyles = (): StyleType => {
             // Base styles are the styles that apply regardless of light mode
             const baseStyles = [
                 styles.textarea,
@@ -284,7 +284,7 @@ const TextArea = React.forwardRef<HTMLTextAreaElement, TextAreaProps>(
                     data-testid={testId}
                     ref={ref}
                     className={className}
-                    style={[...getStyles(), style]}
+                    style={[getStyles(), style]}
                     value={value}
                     onChange={handleChange}
                     placeholder={placeholder}
@@ -338,7 +338,9 @@ const styles = StyleSheet.create({
         ":focus-visible": {
             borderColor: color.blue,
             outline: `1px solid ${color.blue}`,
-            outlineOffset: 0, // Explicitly set outline offset to 0 because Safari sets a default offset
+            // Negative outline offset so it focus outline is not cropped off if
+            // an ancestor element has overflow: hidden
+            outlineOffset: "-2px",
         },
     },
     disabled: {
@@ -350,8 +352,8 @@ const styles = StyleSheet.create({
         },
         cursor: "not-allowed",
         ":focus-visible": {
-            outline: "none",
-            boxShadow: `0 0 0 1px ${color.white}, 0 0 0 3px ${color.offBlack32}`,
+            outline: `2px solid ${color.offBlack32}`,
+            outlineOffset: "-3px",
         },
     },
     error: {
@@ -376,10 +378,9 @@ const styles = StyleSheet.create({
     },
     lightFocus: {
         ":focus-visible": {
-            outline: `1px solid ${color.blue}`,
-            outlineOffset: 0, // Explicitly set outline offset to 0 because Safari sets a default offset
-            borderColor: color.blue,
-            boxShadow: `0px 0px 0px 2px ${color.blue}, 0px 0px 0px 3px ${color.white}`,
+            outline: `3px solid ${color.blue}`,
+            outlineOffset: "-4px",
+            borderColor: color.white,
         },
     },
     lightDisabled: {
@@ -392,22 +393,22 @@ const styles = StyleSheet.create({
         cursor: "not-allowed",
         ":focus-visible": {
             borderColor: mix(color.white32, color.blue),
-            outline: "none",
-            boxShadow: `0 0 0 1px ${color.offBlack32}, 0 0 0 3px ${color.fadedBlue}`,
+            outline: `3px solid ${color.fadedBlue}`,
+            outlineOffset: "-4px",
         },
     },
     lightError: {
         background: color.fadedRed8,
-        border: `1px solid ${color.red}`,
-        boxShadow: `0px 0px 0px 1px ${color.red}, 0px 0px 0px 2px ${color.white}`,
+        border: `1px solid ${color.white}`,
+        outline: `2px solid ${color.red}`,
+        outlineOffset: "-3px",
         color: color.offBlack,
         "::placeholder": {
             color: color.offBlack64,
         },
         ":focus-visible": {
-            outlineColor: color.red,
-            borderColor: color.red,
-            boxShadow: `0px 0px 0px 2px ${color.red}, 0px 0px 0px 3px ${color.white}`,
+            outline: `3px solid ${color.red}`,
+            outlineOffset: "-4px",
         },
     },
 });