feat(Link)!: the "href" property is required (#1270)

gravity-ui · Jan 31, 2024 · 8bbedf4 · 8bbedf4
1 parent 2d49002
commit 8bbedf4
Show file tree

Hide file tree

Showing 14 changed files with 113 additions and 133 deletions.
diff --git a/package.json b/package.json
@@ -76,10 +76,10 @@
         "build-storybook": "sb build -c .storybook -o storybook-static",
         "lint:js": "eslint --ext .js,.jsx,.ts,.tsx .",
         "lint:js:fix": "npm run lint:js -- --fix",
-        "lint:styles": "stylelint '{styles,src}/**/*.scss'",
-        "lint:styles:fix": "npm run lint:styles -- --fix",
-        "lint:prettier": "prettier --check '**/*.md'",
-        "lint:prettier:fix": "prettier --write '**/*.md'",
+        "lint:styles": "cross-env stylelint '{styles,src}/**/*.scss'",
+        "lint:styles:fix": "cross-env npm run lint:styles -- --fix",
+        "lint:prettier": "cross-env prettier --check '**/*.md'",
+        "lint:prettier:fix": "cross-env prettier --write '**/*.md'",
         "lint": "run-p lint:*",
         "typecheck": "tsc --noEmit",
         "prepublishOnly": "npm run build",

diff --git a/src/components/Breadcrumbs/Breadcrumbs.scss b/src/components/Breadcrumbs/Breadcrumbs.scss
@@ -1,4 +1,5 @@
 @use '../variables';
+@use '../../../styles/mixins';
 
 $block: '.#{variables.$ns}breadcrumbs';
 
@@ -12,7 +13,17 @@ $block: '.#{variables.$ns}breadcrumbs';
         gap: 4px;
     }
 
-    &__item {
+    &__switcher {
+        @include mixins.button-reset();
+        color: var(--g-color-text-secondary);
+
+        &:focus-visible {
+            outline: 2px solid var(--g-color-line-focus);
+        }
+    }
+
+    &__item,
+    &__switcher {
         flex-shrink: 1;
         display: inline-block;
         overflow: hidden;

diff --git a/src/components/Breadcrumbs/Breadcrumbs.tsx b/src/components/Breadcrumbs/Breadcrumbs.tsx
@@ -12,13 +12,23 @@ import {BreadcrumbsSeparator} from './BreadcrumbsSeparator';
 
 import './Breadcrumbs.scss';
 
-export interface BreadcrumbsItem {
+type BaseBreadcrumbsItem = {
     text: string;
-    action: (event: React.MouseEvent<HTMLElement, MouseEvent> | KeyboardEvent) => void;
-    href?: string;
     items?: BreadcrumbsItem[];
     title?: string;
-}
+};
+
+type LinkBreadcrumbsItem = {
+    href: string;
+    action?: (event: React.MouseEvent<HTMLElement, MouseEvent> | KeyboardEvent) => void;
+} & BaseBreadcrumbsItem;
+
+type ButtonBreadcrumbsItem = {
+    href?: undefined;
+    action: (event: React.MouseEvent<HTMLElement, MouseEvent> | KeyboardEvent) => void;
+} & BaseBreadcrumbsItem;
+
+export type BreadcrumbsItem = LinkBreadcrumbsItem | ButtonBreadcrumbsItem;
 
 export interface BreadcrumbsProps<T extends BreadcrumbsItem = BreadcrumbsItem> extends QAProps {
     items: T[];

diff --git a/src/components/Breadcrumbs/BreadcrumbsButton.tsx b/src/components/Breadcrumbs/BreadcrumbsButton.tsx
@@ -0,0 +1,13 @@
+import React from 'react';
+
+import {block} from '../utils/cn';
+
+const b = block('breadcrumbs');
+
+export function BreadcrumbsButton(props: {
+    title: string;
+    onClick: React.MouseEventHandler<HTMLElement>;
+    children: React.ReactNode;
+}) {
+    return <button {...props} type="button" className={b('switcher', {more: true})} />;
+}
diff --git a/src/components/Breadcrumbs/BreadcrumbsItem.tsx b/src/components/Breadcrumbs/BreadcrumbsItem.tsx
@@ -4,6 +4,7 @@ import {Link} from '../Link';
 import {block} from '../utils/cn';
 
 import type {BreadcrumbsProps, BreadcrumbsItem as IBreadcrumbsItem} from './Breadcrumbs';
+import {BreadcrumbsButton} from './BreadcrumbsButton';
 
 interface Props<T extends IBreadcrumbsItem = IBreadcrumbsItem> {
     data: T;
@@ -22,27 +23,36 @@ function Item<T extends IBreadcrumbsItem = IBreadcrumbsItem>({
     isPrevCurrent,
     renderItem,
 }: Props<T>) {
-    const {text, title, href, action} = data;
-    const itemTitle = title || text;
+    const itemTitle = data.title || data.text;
+
+    const item = renderItem ? renderItem(data, isCurrent, isPrevCurrent) : data.text;
 
     if (isPrevCurrent || !isCurrent) {
+        if (data.href !== undefined) {
+            return (
+                <Link
+                    key={data.text}
+                    view="secondary"
+                    href={data.href}
+                    title={itemTitle}
+                    onClick={data.action}
+                    className={b('item', {'prev-current': isPrevCurrent})}
+                >
+                    {item}
+                </Link>
+            );
+        }
+
         return (
-            <Link
-                key={text}
-                view="secondary"
-                href={href}
-                title={itemTitle}
-                onClick={action}
-                className={b('item', {'prev-current': isPrevCurrent})}
-            >
-                {renderItem ? renderItem(data, isCurrent, isPrevCurrent) : text}
-            </Link>
+            <BreadcrumbsButton key={data.text} title={itemTitle} onClick={data.action}>
+                {item}
+            </BreadcrumbsButton>
         );
     }
 
     return (
         <div title={itemTitle} className={b('item', {current: true})}>
-            {renderItem ? renderItem(data, isCurrent, isPrevCurrent) : text}
+            {item}
         </div>
     );
 }

diff --git a/src/components/Breadcrumbs/BreadcrumbsMore.tsx b/src/components/Breadcrumbs/BreadcrumbsMore.tsx
@@ -1,10 +1,10 @@
 import React from 'react';
 
 import {DropdownMenu} from '../DropdownMenu';
-import {Link} from '../Link';
 import {block} from '../utils/cn';
 
 import type {BreadcrumbsProps} from './Breadcrumbs';
+import {BreadcrumbsButton} from './BreadcrumbsButton';
 import i18n from './i18n';
 
 interface Props extends Pick<BreadcrumbsProps, 'popupPlacement' | 'popupStyle' | 'items'> {}
@@ -22,14 +22,9 @@ export function BreadcrumbsMore({popupStyle, popupPlacement, items}: Props) {
                 placement: popupPlacement,
             }}
             renderSwitcher={({onClick}) => (
-                <Link
-                    view="secondary"
-                    title={i18n('label_more')}
-                    className={b('item', {more: true})}
-                    onClick={onClick}
-                >
+                <BreadcrumbsButton title={i18n('label_more')} onClick={onClick}>
                     ...
-                </Link>
+                </BreadcrumbsButton>
             )}
         />
     );

diff --git a/src/components/Link/Link.tsx b/src/components/Link/Link.tsx
@@ -12,22 +12,20 @@ export interface LinkProps extends DOMProps, QAProps {
     view?: LinkView;
     visitable?: boolean;
     title?: string;
-    href?: string;
+    href: string;
     target?: string;
     rel?: string;
     id?: string;
     children?: React.ReactNode;
-    onClick?: React.MouseEventHandler<HTMLAnchorElement | HTMLSpanElement>;
-    onFocus?: React.FocusEventHandler<HTMLAnchorElement | HTMLSpanElement>;
-    onBlur?: React.FocusEventHandler<HTMLAnchorElement | HTMLSpanElement>;
-    extraProps?:
-        | React.AnchorHTMLAttributes<HTMLAnchorElement>
-        | React.HTMLAttributes<HTMLSpanElement>;
+    onClick?: React.MouseEventHandler<HTMLAnchorElement>;
+    onFocus?: React.FocusEventHandler<HTMLAnchorElement>;
+    onBlur?: React.FocusEventHandler<HTMLAnchorElement>;
+    extraProps?: React.AnchorHTMLAttributes<HTMLAnchorElement>;
 }
 
 const b = block('link');
 
-export const Link = React.forwardRef<HTMLElement, LinkProps>(function Link(
+export const Link = React.forwardRef<HTMLAnchorElement, LinkProps>(function Link(
     {
         view = 'normal',
         visitable,
@@ -67,33 +65,11 @@ export const Link = React.forwardRef<HTMLElement, LinkProps>(function Link(
         'data-qa': qa,
     };
 
-    if (typeof href === 'string') {
-        const relProp = target === '_blank' && !rel ? 'noopener noreferrer' : rel;
+    const relProp = target === '_blank' && !rel ? 'noopener noreferrer' : rel;
 
-        return (
-            <a
-                {...(extraProps as React.AnchorHTMLAttributes<HTMLAnchorElement>)}
-                {...commonProps}
-                ref={ref as React.Ref<HTMLAnchorElement>}
-                href={href}
-                target={target}
-                rel={relProp}
-            >
-                {children}
-            </a>
-        );
-    } else {
-        return (
-            <span
-                {...(extraProps as React.HTMLAttributes<HTMLSpanElement>)}
-                {...commonProps}
-                ref={ref as React.Ref<HTMLSpanElement>}
-                // as this element has onClick handler
-                // eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex
-                tabIndex={0}
-            >
-                {children}
-            </span>
-        );
-    }
+    return (
+        <a {...extraProps} {...commonProps} ref={ref} href={href} target={target} rel={relProp}>
+            {children}
+        </a>
+    );
 });
diff --git a/src/components/Link/README.md b/src/components/Link/README.md
@@ -110,24 +110,21 @@ LANDING_BLOCK-->
 
 ## Href
 
-The `href` property is optional. If it is skipped, the `Link` will behave like a `Button`.
+The `href` property is required.
 
 <!--LANDING_BLOCK
 <ExampleBlock
     code={`
 <Link href="#">Link with href</Link>
-<Link>Link without href</Link>
 `}>
     <UIKit.Link href="#">Link with href</UIKit.Link>
-    <UIKit.Link>Link without href</UIKit.Link>
 </ExampleBlock>
 LANDING_BLOCK-->
 
 <!--GITHUB_BLOCK-->
 
 ```tsx
 <Link href="#">Link with href</Link>
-<Link>Link without href</Link>
 ```
 
 <!--/GITHUB_BLOCK-->
@@ -140,17 +137,17 @@ A `Link` can be used both as an independent text element and as part of the text
 <ExampleBlock
     code={`
 <Text>
-    <Link>what roles are active in the service</Link>
+    <Link href="#">what roles are active in the service</Link>
 </Text>
 <Text>
-    Currently, this role can only be assigned to a <Link>folder</Link> or <Link>cloud</Link>
+    Currently, this role can only be assigned to a <Link href="#">folder</Link> or <Link href="#">cloud</Link>
 </Text>
 `}>
     <UIKit.Text>
-        <UIKit.Link>what roles are active in the service</UIKit.Link>
+        <UIKit.Link href="#">what roles are active in the service</UIKit.Link>
     </UIKit.Text>
     <UIKit.Text>
-        Currently, this role can only be assigned to a <UIKit.Link>folder</UIKit.Link> or <UIKit.Link>cloud</UIKit.Link>
+        Currently, this role can only be assigned to a <UIKit.Link href="#">folder</UIKit.Link> or <UIKit.Link href="#">cloud</UIKit.Link>
     </UIKit.Text>
 </ExampleBlock>
 LANDING_BLOCK-->
@@ -159,32 +156,32 @@ LANDING_BLOCK-->
 
 ```tsx
 <Text>
-    <Link>What roles are available in the service</Link>
+    <Link href="#">What roles are available in the service</Link>
 </Text>
 <Text>
-    Currently, this role can only be assigned to a <Link>folder</Link> or <Link>cloud</Link>
+    Currently, this role can only be assigned to a <Link href="#">folder</Link> or <Link href="#">cloud</Link>
 </Text>
 ```
 
 <!--/GITHUB_BLOCK-->
 
 ## Properties
 
-| Name       | Description                                |                                     Type                                     |  Default   |
-| :--------- | :----------------------------------------- | :--------------------------------------------------------------------------: | :--------: |
-| view       | Link appearance                            |                    `"normal" \| "primary" \| "secondary"`                    | `"normal"` |
-| visitable  | Display `:visitable` CSS state             |                            `boolean \| undefined`                            |
-| href       | HTML `href` attribute                      |                            `string \| undefined`                             |
-| target     | HTML `target` attribute                    |                            `string \| undefined`                             |
-| rel        | HTML `rel` attribute                       |                            `string \| undefined`                             |
-| title      | HTML `title` attribute                     |                            `string \| undefined`                             |
-| children   | Link content                               |                              `React.ReactNode`                               |
-| extraProps | Any additional props                       |                            `Record \| undefined`                             |
-| onClick    | `click` event handler                      | `React.MouseEventHandler<HTMLAnchorElement \| HTMLSpanElement> \| undefined` |
-| onFocus    | `focus` event handler                      | `React.FocusEventHandler<HTMLAnchorElement \| HTMLSpanElement> \| undefined` |
-| onBlur     | `blur` event handler                       | `React.FocusEventHandler<HTMLAnchorElement \| HTMLSpanElement> \| undefined` |
-| id         | HTML `id` attribute                        |                            `string \| undefined`                             |
-| style      | HTML `style` attribute                     |                      `React.CSSProperties \| undefined`                      |
-| className  | HTML `class` attribute                     |                            `string \| undefined`                             |
-| qa         | HTML `data-qa` attribute, used for testing |                            `string \| undefined`                             |
-| ref        | React ref to Link DOM node                 |                `React.ForwardedRef<HTMLElement> \| undefined`                |
+| Name       | Description                                |                           Type                            |  Default   |
+| :--------- | :----------------------------------------- | :-------------------------------------------------------: | :--------: |
+| view       | Link appearance                            |          `"normal" \| "primary" \| "secondary"`           | `"normal"` |
+| visitable  | Display `:visitable` CSS state             |                  `boolean \| undefined`                   |
+| href       | HTML `href` attribute                      |                   `string \| undefined`                   |
+| target     | HTML `target` attribute                    |                   `string \| undefined`                   |
+| rel        | HTML `rel` attribute                       |                   `string \| undefined`                   |
+| title      | HTML `title` attribute                     |                   `string \| undefined`                   |
+| children   | Link content                               |                     `React.ReactNode`                     |
+| extraProps | Any additional props                       |                   `Record \| undefined`                   |
+| onClick    | `click` event handler                      | `React.MouseEventHandler<HTMLAnchorElement> \| undefined` |
+| onFocus    | `focus` event handler                      | `React.FocusEventHandler<HTMLAnchorElement> \| undefined` |
+| onBlur     | `blur` event handler                       | `React.FocusEventHandler<HTMLAnchorElement> \| undefined` |
+| id         | HTML `id` attribute                        |                   `string \| undefined`                   |
+| style      | HTML `style` attribute                     |            `React.CSSProperties \| undefined`             |
+| className  | HTML `class` attribute                     |                   `string \| undefined`                   |
+| qa         | HTML `data-qa` attribute, used for testing |                   `string \| undefined`                   |
+| ref        | React ref to Link DOM node                 |   `React.ForwardedRef<HTMLAnchorElement> \| undefined`    |
diff --git a/src/components/Link/__stories__/Link.new.stories.tsx b/src/components/Link/__stories__/Link.new.stories.tsx
@@ -3,6 +3,7 @@ import React from 'react';
 import type {Meta, StoryFn} from '@storybook/react';
 
 import {Link} from '../Link';
+import type {LinkProps} from '../Link';
 
 export default {
     title: 'Components/Navigation/Link',
@@ -45,7 +46,7 @@ export default {
     },
 } as Meta;
 
-export const Playground: StoryFn = (args) => {
+export const Playground: StoryFn<LinkProps> = (args) => {
     return <Link {...args} />;
 };
 Playground.storyName = 'Link';
diff --git a/src/components/Link/__stories__/LinkShowcase.tsx b/src/components/Link/__stories__/LinkShowcase.tsx
@@ -1,14 +1,14 @@
 import React from 'react';
 
 import {Showcase} from '../../../demo/Showcase';
-import {ShowcaseGrid} from '../../../demo/ShowcaseGrid';
+import {ShowcaseGrid} from '../../../demo/ShowcaseGrid/ShowcaseGrid';
 import {Link} from '../Link';
 
 export function LinkShowcase() {
     return (
         <Showcase title="Link">
             <ShowcaseGrid
-                rowKey="href"
+                rowKey="target"
                 component={Link}
                 propsCombinations={{
                     view: [
@@ -25,16 +25,6 @@ export function LinkShowcase() {
                             value: 'secondary',
                         },
                     ],
-                    href: [
-                        {
-                            name: 'With href',
-                            value: '#',
-                        },
-                        {
-                            name: 'Without href',
-                            value: undefined,
-                        },
-                    ],
                     visitable: [
                         {
                             name: 'Not visitable',
@@ -58,6 +48,7 @@ export function LinkShowcase() {
                 }}
                 staticProps={{
                     children: 'Link',
+                    href: '#',
                 }}
             />
         </Showcase>