Better styling for multi-line links (#146)

LatitudeFinancialOSS · Sep 7, 2020 · 968be95 · 968be95 · vercel · Sep 7, 2020
1 parent 311ac63
commit 968be95
Show file tree

Hide file tree

Showing 5 changed files with 249 additions and 198 deletions.
diff --git a/src/components/Link.js b/src/components/Link.js
@@ -114,19 +114,33 @@ function Link(props) {
     },
     theme
   );
-  const { InternalLink, isLinkInternal } = useContext(LinkContext);
-  const css = useResponsivePropsCSS(mergedProps, DEFAULT_PROPS, {
+  const getAppearanceFromVariant = (variant) =>
+    props.appearance ??
+    (["blue-button", "white-button", "green-button"].includes(variant)
+      ? "primary-button"
+      : variant === "black-button"
+      ? "secondary-button"
+      : "text");
+  const anchorCSS = useResponsivePropsCSS(mergedProps, DEFAULT_PROPS, {
     variant: (_, theme, bp) => {
       const variant = props.variant ?? variantMap[bp];
-      const appearance =
-        props.appearance ??
-        (["blue-button", "white-button", "green-button"].includes(variant)
-          ? "primary-button"
-          : variant === "black-button"
-          ? "secondary-button"
-          : "text");
+      const appearance = getAppearanceFromVariant(variant);
 
       return theme.link.getCSS({
+        targetElement: "anchor",
+        appearance,
+        __internal__keyboardFocus,
+      });
+    },
+    margin: responsiveMargin,
+  });
+  const spanCSS = useResponsivePropsCSS(mergedProps, DEFAULT_PROPS, {
+    variant: (_, theme, bp) => {
+      const variant = props.variant ?? variantMap[bp];
+      const appearance = getAppearanceFromVariant(variant);
+
+      return theme.link.getCSS({
+        targetElement: "span",
         appearance,
         variant,
         buttonTheme: theme.button,
@@ -135,11 +149,11 @@ function Link(props) {
         __internal__active,
       });
     },
-    margin: responsiveMargin,
     padding: responsivePadding,
     width: responsiveSize("width"),
   });
 
+  const { InternalLink, isLinkInternal } = useContext(LinkContext);
   const newTabProps = newTab
     ? {
         target: "_blank",
@@ -165,29 +179,29 @@ function Link(props) {
     return (
       <InternalLink
         className={analyticsClassName}
-        css={css}
+        css={anchorCSS}
         to={href}
         title={title}
         state={state}
         onClick={onClick}
         data-testid={testId}
       >
-        {children}
+        <span css={spanCSS}>{children}</span>
       </InternalLink>
     );
   }
 
   return (
     <a
       className={analyticsClassName}
-      css={css}
+      css={anchorCSS}
       href={href}
       title={title}
       onClick={onClick}
       data-testid={testId}
       {...newTabProps}
     >
-      {children}
+      <span css={spanCSS}>{children}</span>
     </a>
   );
 }

diff --git a/src/components/Link.test.js b/src/components/Link.test.js
@@ -12,13 +12,18 @@ describe("Link", () => {
       </Link>
     );
 
-    const link = screen.getByText("Terms and Conditions");
+    const span = screen.getByText("Terms and Conditions");
+    const link = span.parentElement;
 
     expect(link.tagName).toBe("A");
     expect(link).not.toHaveAttribute("target");
     expect(link).toHaveAttribute("href", "/terms");
     expect(link).toHaveStyle({
       textDecoration: "none",
+      borderRadius: "4px",
+      outline: 0,
+    });
+    expect(span).toHaveStyle({
       borderBottomWidth: "1px",
       borderBottomStyle: "solid",
       borderBottomColor: "rgba(0,70,170,0.5)",
@@ -36,7 +41,8 @@ describe("Link", () => {
       </Link>
     );
 
-    const link = screen.getByText("Terms and Conditions");
+    const span = screen.getByText("Terms and Conditions");
+    const link = span.parentElement;
 
     expect(link).toHaveAttribute("target", "_blank");
     expect(link).toHaveAttribute("rel", "noreferrer");
@@ -54,9 +60,9 @@ describe("Link", () => {
       </Link>
     );
 
-    const link = screen.getByText("Terms and Conditions");
+    const span = screen.getByText("Terms and Conditions");
 
-    expect(link).toHaveStyle({
+    expect(span).toHaveStyle({
       backgroundColor: "#ffffff",
       color: "#006aff",
     });
@@ -69,9 +75,9 @@ describe("Link", () => {
       </Link>
     );
 
-    const link = screen.getByText("Terms and Conditions");
+    const span = screen.getByText("Terms and Conditions");
 
-    expect(link).toHaveStyle({
+    expect(span).toHaveStyle({
       color: "#ffffff",
       borderBottomColor: "rgba(255,255,255,0.5)",
     });
@@ -84,9 +90,9 @@ describe("Link", () => {
       </Link>
     );
 
-    const link = screen.getByText("Terms and Conditions");
+    const span = screen.getByText("Terms and Conditions");
 
-    expect(link).toHaveStyle({
+    expect(span).toHaveStyle({
       backgroundColor: "transparent",
       color: "#006aff",
       borderWidth: "1px",
@@ -102,9 +108,9 @@ describe("Link", () => {
       </Link>
     );
 
-    const link = screen.getByText("Terms and Conditions");
+    const span = screen.getByText("Terms and Conditions");
 
-    expect(link).toHaveStyle({
+    expect(span).toHaveStyle({
       backgroundColor: "transparent",
       color: "#000000",
       borderWidth: "1px",
@@ -122,9 +128,9 @@ describe("Link", () => {
       </Container>
     );
 
-    const link = screen.getByText("Terms and Conditions");
+    const span = screen.getByText("Terms and Conditions");
 
-    expect(link).toHaveStyle({
+    expect(span).toHaveStyle({
       color: "#ffffff",
       borderBottomColor: "rgba(255,255,255,0.5)",
     });
@@ -141,9 +147,9 @@ describe("Link", () => {
       </Container>
     );
 
-    const link = screen.getByText("Terms and Conditions");
+    const span = screen.getByText("Terms and Conditions");
 
-    expect(link).toHaveStyle({
+    expect(span).toHaveStyle({
       color: "#ffffff",
       borderBottomColor: "rgba(255,255,255,0.5)",
     });
@@ -156,7 +162,8 @@ describe("Link", () => {
       </Link>
     );
 
-    const link = screen.getByText("Terms and Conditions");
+    const span = screen.getByText("Terms and Conditions");
+    const link = span.parentElement;
 
     expect(link).toHaveStyle({
       margin: "4px 8px 12px",
@@ -170,9 +177,9 @@ describe("Link", () => {
       </Link>
     );
 
-    const link = screen.getByText("Terms and Conditions");
+    const span = screen.getByText("Terms and Conditions");
 
-    expect(link).toHaveStyle({
+    expect(span).toHaveStyle({
       padding: "12px 24px",
     });
   });
@@ -189,9 +196,9 @@ describe("Link", () => {
       </Link>
     );
 
-    const link = screen.getByText("Terms and Conditions");
+    const span = screen.getByText("Terms and Conditions");
 
-    expect(link).toHaveStyle({
+    expect(span).toHaveStyle({
       width: "300px",
     });
   });
@@ -207,7 +214,8 @@ describe("Link", () => {
       </Link>
     );
 
-    const link = screen.getByText("Terms and Conditions");
+    const span = screen.getByText("Terms and Conditions");
+    const link = span.parentElement;
 
     userEvent.click(link);
 
@@ -221,7 +229,8 @@ describe("Link", () => {
       </Link>
     );
 
-    const link = screen.getByText("Terms and Conditions");
+    const span = screen.getByText("Terms and Conditions");
+    const link = span.parentElement;
 
     expect(link).toHaveClass("track-me");
   });

diff --git a/src/components/Message.test.js b/src/components/Message.test.js
@@ -73,7 +73,9 @@ describe("Message", () => {
       </Message>
     );
 
-    expect(screen.getByRole("link", { name: "Explore how" })).toHaveStyle({
+    expect(
+      screen.getByRole("link", { name: "Explore how" }).firstChild
+    ).toHaveStyle({
       backgroundColor: "transparent",
       color: "#ffffff",
       borderWidth: "1px",