From 84264047155dae095d3ba0b38468a7a43f3a0cf0 Mon Sep 17 00:00:00 2001
From: LakeVostok <lakevostok@yandex.ru>
Date: Wed, 24 Jan 2024 21:51:28 +0300
Subject: [PATCH] chore(Link)!: the "href" property is required

---
 package.json                                  |  8 +--
 src/components/Breadcrumbs/Breadcrumbs.scss   |  9 +++-
 src/components/Breadcrumbs/Breadcrumbs.tsx    | 15 ++++--
 .../Breadcrumbs/BreadcrumbsButton.tsx         | 13 +++++
 .../Breadcrumbs/BreadcrumbsItem.tsx           | 33 ++++++++----
 .../Breadcrumbs/BreadcrumbsMore.tsx           | 11 ++--
 src/components/Link/Link.tsx                  | 48 +++++------------
 src/components/Link/README.md                 | 53 +++++++++----------
 .../Link/__stories__/Link.new.stories.tsx     |  4 +-
 .../Link/__stories__/Link.stories.tsx         |  3 +-
 .../Link/__stories__/LinkShowcase.tsx         | 15 ++----
 src/components/Link/__tests__/Link.test.tsx   |  9 ----
 .../Popover/__stories__/Popover.stories.tsx   |  4 --
 .../Popover/__tests__/Popover.test.tsx        | 13 +----
 .../Popover/components/Links/Links.tsx        |  2 +-
 15 files changed, 107 insertions(+), 133 deletions(-)
 create mode 100644 src/components/Breadcrumbs/BreadcrumbsButton.tsx

diff --git a/package.json b/package.json
index b774007bec..3bc8070938 100644
--- 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
index b4eeff13fe..0be639096f 100644
--- 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';
 
@@ -11,7 +12,13 @@ $block: '.#{variables.$ns}breadcrumbs';
         overflow: hidden;
     }
 
-    &__item {
+    &__switcher {
+        @include mixins.button-reset();
+        color: var(--g-color-text-secondary);
+    }
+
+    &__item,
+    &__switcher {
         flex-shrink: 1;
         padding: 4px 8px;
         display: inline-block;
diff --git a/src/components/Breadcrumbs/Breadcrumbs.tsx b/src/components/Breadcrumbs/Breadcrumbs.tsx
index 4ba7a894aa..4aeb2f3133 100644
--- a/src/components/Breadcrumbs/Breadcrumbs.tsx
+++ b/src/components/Breadcrumbs/Breadcrumbs.tsx
@@ -12,13 +12,22 @@ 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;
+} & BaseBreadcrumbsItem;
+
+type ButtonBreadcrumbsItem = {
+    href?: undefined;
+} & 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
new file mode 100644
index 0000000000..62ed662f8d
--- /dev/null
+++ 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} className={b('switcher', {more: true})} />;
+}
diff --git a/src/components/Breadcrumbs/BreadcrumbsItem.tsx b/src/components/Breadcrumbs/BreadcrumbsItem.tsx
index d5d67d62b6..314dc3cb00 100644
--- 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;
@@ -25,24 +26,34 @@ function Item<T extends IBreadcrumbsItem = IBreadcrumbsItem>({
     const {text, title, href, action} = data;
     const itemTitle = title || text;
 
+    const item = renderItem ? renderItem(data, isCurrent, isPrevCurrent) : text;
+
     if (isPrevCurrent || !isCurrent) {
+        if (href) {
+            return (
+                <Link
+                    key={text}
+                    view="secondary"
+                    href={href}
+                    title={itemTitle}
+                    onClick={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={text} title={itemTitle} onClick={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
index d4db1684fa..6effc9708b 100644
--- 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
index 4503356bc4..7118386124 100644
--- 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
index 1a5299a537..c5ad002593 100644
--- a/src/components/Link/README.md
+++ b/src/components/Link/README.md
@@ -110,16 +110,14 @@ 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-->
 
@@ -127,7 +125,6 @@ LANDING_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,10 +156,10 @@ 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>
 ```
 
@@ -170,21 +167,21 @@ LANDING_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
index 40a4e82f2f..26602badc0 100644
--- a/src/components/Link/__stories__/Link.new.stories.tsx
+++ b/src/components/Link/__stories__/Link.new.stories.tsx
@@ -2,7 +2,7 @@ import React from 'react';
 
 import type {Meta, StoryFn} from '@storybook/react';
 
-import {Link} from '../Link';
+import {Link, type LinkProps} from '../Link';
 
 export default {
     title: 'Components/Navigation/Link',
@@ -45,7 +45,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__/Link.stories.tsx b/src/components/Link/__stories__/Link.stories.tsx
index 29f742927d..b44cf09b9d 100644
--- a/src/components/Link/__stories__/Link.stories.tsx
+++ b/src/components/Link/__stories__/Link.stories.tsx
@@ -2,8 +2,7 @@ import React from 'react';
 
 import type {Meta, StoryFn} from '@storybook/react';
 
-import {Link} from '../Link';
-import type {LinkProps} from '../Link';
+import {Link, type LinkProps} from '../Link';
 import {LinkShowcase} from '../__stories__/LinkShowcase';
 
 export default {
diff --git a/src/components/Link/__stories__/LinkShowcase.tsx b/src/components/Link/__stories__/LinkShowcase.tsx
index b1a52d617a..a13e8341f7 100644
--- 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>
diff --git a/src/components/Link/__tests__/Link.test.tsx b/src/components/Link/__tests__/Link.test.tsx
index fb1232a22e..13759841de 100644
--- a/src/components/Link/__tests__/Link.test.tsx
+++ b/src/components/Link/__tests__/Link.test.tsx
@@ -12,15 +12,6 @@ describe('Link', () => {
         expect(link).toBeInTheDocument();
     });
 
-    test('Link is span', () => {
-        render(<Link qa="test" />);
-
-        // btw there should be role attribute at span
-        const link = screen.getByTestId('test');
-
-        expect(link.tagName).toBe('SPAN');
-    });
-
     test('rel attribute is present', () => {
         render(<Link href="#" target="_blank" />);
 
diff --git a/src/components/Popover/__stories__/Popover.stories.tsx b/src/components/Popover/__stories__/Popover.stories.tsx
index ba69d27b3d..294ffef724 100644
--- a/src/components/Popover/__stories__/Popover.stories.tsx
+++ b/src/components/Popover/__stories__/Popover.stories.tsx
@@ -151,10 +151,6 @@ FullFeatured.args = {
             text: 'Link with a href',
             href: 'https://yandex.ru',
         },
-        {
-            text: 'Link with an onClick handler',
-            onClick: () => alert('Link was clicked'),
-        },
     ],
     className: 'demo-icon-tooltip',
     openOnHover: false,
diff --git a/src/components/Popover/__tests__/Popover.test.tsx b/src/components/Popover/__tests__/Popover.test.tsx
index 5d6103f27f..7433668be4 100644
--- a/src/components/Popover/__tests__/Popover.test.tsx
+++ b/src/components/Popover/__tests__/Popover.test.tsx
@@ -239,20 +239,14 @@ test('Renders with html content', () => {
 });
 
 test('Renders with links', () => {
-    const onLinkClick = jest.fn();
-
     const linkWithHrefConfig = {
         text: 'Link with a href',
         href: 'https://yandex.ru',
     };
-    const linkWithClickHandlerConfig = {
-        text: 'Link with an onClick handler',
-        onClick: onLinkClick,
-    };
 
     render(
         renderPopover({
-            links: [linkWithHrefConfig, linkWithClickHandlerConfig],
+            links: [linkWithHrefConfig],
         }),
     );
 
@@ -260,12 +254,7 @@ test('Renders with links', () => {
     fireEvent.click(popoverTrigger);
 
     const linkWithHref = screen.getByText(linkWithHrefConfig.text);
-    const linkWithClickHandler = screen.getByText(linkWithClickHandlerConfig.text);
     expect(linkWithHref).toBeInTheDocument();
-    expect(linkWithClickHandler).toBeInTheDocument();
-
-    fireEvent.click(linkWithClickHandler);
-    expect(onLinkClick).toHaveBeenCalledTimes(1);
 });
 
 test('Renders with buttons', () => {
diff --git a/src/components/Popover/components/Links/Links.tsx b/src/components/Popover/components/Links/Links.tsx
index 4119a6abc3..5c34806621 100644
--- a/src/components/Popover/components/Links/Links.tsx
+++ b/src/components/Popover/components/Links/Links.tsx
@@ -6,7 +6,7 @@ import {cnPopover} from '../../Popover.classname';
 export type LinksProps = {
     links: Array<{
         text: string;
-        href?: string;
+        href: string;
         target?: '_self' | '_blank';
         onClick?: (event: React.MouseEvent<HTMLAnchorElement>) => void;
     }>;