fix(Link): address problems with link color inheritance

- properly apply standalone and non-standalone colors - add notes on runtime warnings to generate - add missing stories to demonstrate color inheritance
chanzuckerberg · Jun 6, 2024 · fb7fc3c · fb7fc3c
1 parent 0f9c2b3
commit fb7fc3c
Show file tree

Hide file tree

Showing 6 changed files with 336 additions and 152 deletions.
diff --git a/src/components/AppNotification/AppNotification.stories.tsx b/src/components/AppNotification/AppNotification.stories.tsx
@@ -6,6 +6,8 @@ import React from 'react';
 import { AppNotification } from './AppNotification';
 import { ButtonV2 as Button } from '../Button';
 import { ButtonGroupV2 as ButtonGroup } from '../ButtonGroup';
+import { LinkV2 as Link } from '../Link';
+import Text from '../Text';
 
 export default {
   title: 'Components/V2/AppNotification',
@@ -52,6 +54,20 @@ export const WithControls: StoryObj<Args> = {
   },
 };
 
+export const WithLinkInSubtitle: StoryObj<Args> = {
+  args: {
+    subTitle: (
+      <Text as="span">
+        Some text with a{' '}
+        <Link href="https://example.com/" variant="inverse">
+          link
+        </Link>{' '}
+        in.
+      </Text>
+    ),
+  },
+};
+
 export const LightColor: StoryObj<Args> = {
   args: {
     color: 'light',

diff --git a/src/components/AppNotification/__snapshots__/AppNotification.test.ts.snap b/src/components/AppNotification/__snapshots__/AppNotification.test.ts.snap
@@ -282,3 +282,43 @@ exports[`<AppNotification /> WithDismissAndControls story renders snapshot 1`] =
   </div>
 </div>
 `;
+
+exports[`<AppNotification /> WithLinkInSubtitle story renders snapshot 1`] = `
+<div
+  class="app-notification app-notification--color-dark"
+  role="status"
+>
+  <div
+    class="app-notification__content"
+  >
+    <section>
+      <div
+        class="text text--headline-sm-bold app-notification__title"
+      >
+        This is an AppNotification title
+      </div>
+      <p
+        class="text text--body-md app-notification__sub-title"
+      >
+        <span
+          class="text text--body-md"
+        >
+          Some text with a
+           
+          <a
+            class="link link--emphasis-default link--size-md link--variant-inverse"
+            href="https://example.com/"
+          >
+            link
+          </a>
+           
+          in.
+        </span>
+      </p>
+      <div
+        class="app-notification__actions"
+      />
+    </section>
+  </div>
+</div>
+`;
diff --git a/src/components/Link/Link-v2.module.css b/src/components/Link/Link-v2.module.css
@@ -8,7 +8,6 @@
  */
 
 .link {
-  color: var(--eds-theme-color-text-utility-default-primary);
   display: inline;
 
   /* TODO: verify the way to sync type tokens and one-off treatments */
@@ -17,7 +16,7 @@
 }
 
 /**
- * Sub-components
+ * Sub-components (only should apply to standalone links
  */
 .link__icon {
   padding-left: 0.25rem;
@@ -37,6 +36,15 @@
 .link--context-standalone {
   display: block;
 
+  &.link--emphasis-default {
+    color: var(--eds-theme-color-text-utility-default-primary);
+  }
+
+  &.link--emphasis-low {
+    color: var(--eds-theme-color-text-utility-default-primary);
+    text-decoration: none;
+  }
+
   /**
    * Sizes - using the presets for type ramp matching body-*
    */
@@ -64,31 +72,25 @@
 /**
  * Emphasis
  */
-.link--emphasis-high {
-  color: var(--eds-theme-color-text-utility-interactive-secondary);
+.link--emphasis-default {
+  color: inherit;
 }
 
-.link--emphasis-low {
-  color: var(--eds-theme-color-text-utility-default-primary);
-  text-decoration: none;
+.link--emphasis-high {
+  color: var(--eds-theme-color-text-utility-interactive-secondary);
 }
 
 /**
- * Variants
+ * Inverse variant
  */
-.link.link--variant-inverse {
-  color: var(--eds-theme-color-text-utility-inverse);
-
-  &:active,
-  &:visited {
+.link.link--variant-inverse:hover,
+.link.link--variant-inverse:active {
     color: var(--eds-theme-color-text-utility-inverse);
-  }
 }
 
 /**
  * States
  */
-
 .link:hover {
   color: var(--eds-theme-color-text-utility-interactive-secondary-hover);
 }

diff --git a/src/components/Link/Link-v2.stories.tsx b/src/components/Link/Link-v2.stories.tsx
@@ -12,16 +12,22 @@ export default {
   args: {
     children: 'Link',
     size: 'lg',
-    href: 'https://go.czi.team/eds',
+    href: 'http://example.org/',
     // stop link from navigating to another page so we can click the link for testing
     onClick: (event: any) => event.preventDefault(),
   },
+  decorators: [
+    (Story) => <div className="text-utility-default-primary">{Story()}</div>,
+  ],
 } as Meta<Args>;
 
 type Args = React.ComponentProps<typeof Link>;
 
 export const Default: StoryObj<Args> = {};
 
+/**
+ * When using context, you can specify a trailing icon for the link.
+ */
 export const LinkWithChevron: StoryObj<Args> = {
   args: {
     children: 'Default',
@@ -30,6 +36,9 @@ export const LinkWithChevron: StoryObj<Args> = {
   },
 };
 
+/**
+ * When using context, you can specify a trailing icon for the link. When using the open icon, make sure that a new tab/window is opened.
+ */
 export const LinkWithOpenIcon: StoryObj<Args> = {
   args: {
     children: 'Default',
@@ -38,6 +47,9 @@ export const LinkWithOpenIcon: StoryObj<Args> = {
   },
 };
 
+/**
+ * You can add formatting to the link if needed.
+ */
 export const LinkWithFormattedChildren: StoryObj<Args> = {
   args: {
     children: <em>emphasized link</em>,
@@ -46,6 +58,9 @@ export const LinkWithFormattedChildren: StoryObj<Args> = {
   },
 };
 
+/**
+ * Standalone links can have additional emphasis or minimized emphasis applied.
+ */
 export const Emphasis: StoryObj<Args> = {
   args: {
     size: 'md',
@@ -78,43 +93,76 @@ export const Inverse: StoryObj<Args> = {
   // TODO: find a cleaner way to decorate with unavailable tokens using parameters:backgounds:
   decorators: [
     (Story) => (
-      <div className="w-96 bg-[var(--eds-color-blue-850)] p-1 text-center">
+      <div className="w-96 bg-[var(--eds-color-blue-850)] p-1 text-center text-utility-inverse">
         {Story()}
       </div>
     ),
   ],
 };
 
 export const LinkInParagraphContext: StoryObj<ExtendArgs> = {
-  render: (
-    args: React.JSX.IntrinsicAttributes &
-      (LinkV2Props & React.RefAttributes<HTMLAnchorElement>),
-  ) => (
+  render: (args) => (
+    <div>
+      Lorem ipsum dolor sit amet,{' '}
+      <Link {...args} href="https://example.com/">
+        consectetur adipiscing elit
+      </Link>
+      . Morbi porta at ante quis molestie. Nam scelerisque id diam at iaculis.
+      Nullam sit amet iaculis erat. Nulla id tellus ante.{' '}
+      <Link {...args} href="https://example.org">
+        Aliquam pellentesque ipsum sagittis, commodo neque at, ornare est.
+        Maecenas a malesuada sem, vitae euismod erat. Nullam molestie nunc non
+        dui dignissim fermentum.
+      </Link>{' '}
+      Aliquam id volutpat nulla, sed auctor orci. Fusce cursus leo nisi. Fusce
+      vehicula vitae nisl nec ultricies. Cras ut enim nec magna semper egestas.
+      Sed sed quam id nisl bibendum convallis. Proin suscipit, odio{' '}
+      <Link {...args} href="https://example.edu">
+        vel pulvinar
+      </Link>{' '}
+      euismod, risus eros ullamcorper lectus, non blandit nulla dui eget massa.
+    </div>
+  ),
+};
+
+/**
+ * Links will inherit the color from the surrounding text color definition (default emphasis, inline links)
+ */
+export const InheritColor: StoryObj<Args> = {
+  args: {
+    children: 'Inheriting',
+  },
+  render: (args) => (
     <div>
       Lorem ipsum dolor sit amet,{' '}
-      <Link {...args} href="https://go.czi.team/eds">
+      <Link {...args} href="https://example.com/">
         consectetur adipiscing elit
       </Link>
       . Morbi porta at ante quis molestie. Nam scelerisque id diam at iaculis.
       Nullam sit amet iaculis erat. Nulla id tellus ante.{' '}
-      <Link {...args} href="https://go.czi.team/eds">
+      <Link {...args} href="https://example.org">
         Aliquam pellentesque ipsum sagittis, commodo neque at, ornare est.
         Maecenas a malesuada sem, vitae euismod erat. Nullam molestie nunc non
         dui dignissim fermentum.
       </Link>{' '}
       Aliquam id volutpat nulla, sed auctor orci. Fusce cursus leo nisi. Fusce
       vehicula vitae nisl nec ultricies. Cras ut enim nec magna semper egestas.
       Sed sed quam id nisl bibendum convallis. Proin suscipit, odio{' '}
-      <Link {...args} href="https://go.czi.team/eds">
+      <Link {...args} href="https://example.edu">
         vel pulvinar
       </Link>{' '}
       euismod, risus eros ullamcorper lectus, non blandit nulla dui eget massa.
     </div>
   ),
+  decorators: [
+    (Story) => <div className="text-utility-critical">{Story()}</div>,
+  ],
 };
 
-// Here, we introduce a special type extension to LinkProps, then use it in a
-// composed component, to demonstrate the ability to offer custom props to a component
+/**
+ * Here, we introduce a special type extension to LinkProps, then use it in a
+ * composed component, to demonstrate the ability to offer custom props to a component
+ */
 type ExtendArgs = LinkV2Props<{ to: string }>;
 function ExtendedLink(args: ExtendArgs) {
   return (
@@ -150,7 +198,11 @@ export const UsingExtendedLink: StoryObj<ExtendArgs> = {
   render: (args) => (
     <div>
       Lorem ipsum dolor sit amet,{' '}
-      <ExtendedLink {...args} href="https://go.czi.team/eds" to="test">
+      <ExtendedLink
+        {...args}
+        href="https://example.org/extended-link/"
+        to="test"
+      >
         consectetur adipiscing elit
       </ExtendedLink>
       . Morbi porta at ante quis molestie. Nam scelerisque id diam at iaculis.

diff --git a/src/components/Link/Link-v2.tsx b/src/components/Link/Link-v2.tsx
@@ -79,6 +79,8 @@ export const Link = forwardRef<HTMLAnchorElement, LinkV2Props>(
 
     const iconSize = size && (['xl', 'lg'].includes(size) ? '1.5rem' : '1rem');
 
+    // TODO-AH: Inline links cannot be lowEmphasis (add runtime warning)
+    // TODO-AH: chevron-right only allowr when lowEmphasis is used (add runtime warning)
     return (
       <Component className={componentClassName} ref={ref} {...other}>
         {children}