From 501d9056cc57023b7744bae0faf4028d4f85176c Mon Sep 17 00:00:00 2001
From: kirillmohno <kirillmohno@yandex-team.ru>
Date: Tue, 10 Oct 2023 21:02:12 +0300
Subject: [PATCH] fix: add tags and fix Label component

---
 src/components/DropdownMenu/DropdownMenu.tsx  |  2 +-
 src/components/Label/Label.scss               | 15 ++++++++++++
 src/components/Label/Label.tsx                | 23 +++++++++++++++++--
 src/components/Link/Link.tsx                  |  3 ++-
 .../TableColumnSetup/TableColumnSetup.tsx     |  2 +-
 src/components/UserAvatar/UserAvatar.scss     | 10 --------
 src/components/UserAvatar/UserAvatar.tsx      | 15 +++---------
 src/hooks/useOutsideClick/__tests__/Demo.tsx  |  5 ++--
 8 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/src/components/DropdownMenu/DropdownMenu.tsx b/src/components/DropdownMenu/DropdownMenu.tsx
index c08825023b..5021241d00 100644
--- a/src/components/DropdownMenu/DropdownMenu.tsx
+++ b/src/components/DropdownMenu/DropdownMenu.tsx
@@ -137,7 +137,7 @@ const DropdownMenu = <T,>({
 
     return (
         <DropdownMenuContext.Provider value={contextValue}>
-            {/* It is used to handle clicks on the switcher control */}
+            {/* FIXME change to renderProp or provide component's context */}
             {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
             <div
                 ref={anchorRef}
diff --git a/src/components/Label/Label.scss b/src/components/Label/Label.scss
index 0c25875634..bd41224629 100644
--- a/src/components/Label/Label.scss
+++ b/src/components/Label/Label.scss
@@ -14,6 +14,20 @@ $transitionTimingFunction: ease-in-out;
         border: var(--border-size) solid #{$borderColor};
     }
 
+    // focus on interactive label (excluding focus on addon)
+    &:not(#{$disabled})#{$block}_is-interactive:focus:not(
+            :has(#{$block}__addon_interactive:focus)
+        ) {
+        box-shadow: 0 0 0 2px var(--g-color-line-focus);
+    }
+
+    //fallback for old browsers
+    &:not(#{$disabled})#{$block}_is-interactive:focus:not(
+            :has(#{$block}__addon_interactive:focus)
+        ):not(:focus-visible) {
+        box-shadow: none;
+    }
+
     // hover on interactive label (excluding hover on addon)
     &:not(#{$disabled})#{$block}_is-interactive:hover:not(
             :has(#{$block}__addon_interactive:hover)
@@ -146,6 +160,7 @@ $transitionTimingFunction: ease-in-out;
 
     &_is-interactive {
         cursor: pointer;
+        outline: none;
     }
 
     &_theme {
diff --git a/src/components/Label/Label.tsx b/src/components/Label/Label.tsx
index d245460f25..68c2d4bcbb 100644
--- a/src/components/Label/Label.tsx
+++ b/src/components/Label/Label.tsx
@@ -8,6 +8,7 @@ import {ClipboardIcon} from '../ClipboardIcon';
 import {CopyToClipboard, CopyToClipboardStatus} from '../CopyToClipboard';
 import {Icon} from '../Icon';
 import {block} from '../utils/cn';
+import {useActionHandlers} from '../utils/useActionHandlers';
 
 import './Label.scss';
 
@@ -86,6 +87,8 @@ export const Label = React.forwardRef<HTMLDivElement, LabelProps>(function Label
         onClick,
     } = props;
 
+    const actionButtonRef = React.useRef<HTMLButtonElement>(null);
+
     const hasContent = Boolean(children !== '' && React.Children.count(children) > 0);
 
     const typeClose = type === 'close' && hasContent;
@@ -122,12 +125,25 @@ export const Label = React.forwardRef<HTMLDivElement, LabelProps>(function Label
         }
     };
 
+    const handleClick = (event: React.MouseEvent<HTMLDivElement>) => {
+        /**
+         * Triggered only if the handler was triggered on the element itself, and not on the actionButton
+         * It is necessary that keyboard navigation works correctly
+         */
+        if (!actionButtonRef.current?.contains(event.target as Node)) {
+            onClick?.(event);
+        }
+    };
+
+    const {onKeyDown} = useActionHandlers(handleClick);
+
     const renderLabel = (status?: CopyToClipboardStatus) => {
         let actionButton: React.ReactNode;
 
         if (typeCopy) {
             actionButton = (
                 <Button
+                    ref={actionButtonRef}
                     size={buttonSize}
                     extraProps={{'aria-label': copyButtonLabel || undefined}}
                     {...commonActionButtonProps}
@@ -143,6 +159,7 @@ export const Label = React.forwardRef<HTMLDivElement, LabelProps>(function Label
         } else if (typeClose) {
             actionButton = (
                 <Button
+                    ref={actionButtonRef}
                     onClick={onClose ? handleCloseClick : undefined}
                     size={buttonSize}
                     extraProps={{'aria-label': closeButtonLabel || undefined}}
@@ -154,10 +171,12 @@ export const Label = React.forwardRef<HTMLDivElement, LabelProps>(function Label
         }
 
         return (
-            // eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions
             <div
                 ref={ref}
-                onClick={hasOnClick ? onClick : undefined}
+                role={hasOnClick ? 'role' : undefined}
+                tabIndex={hasOnClick ? 0 : undefined}
+                onClick={hasOnClick ? handleClick : undefined}
+                onKeyDown={hasOnClick ? onKeyDown : undefined}
                 className={b(
                     {
                         theme,
diff --git a/src/components/Link/Link.tsx b/src/components/Link/Link.tsx
index db1b13c800..141556f1ff 100644
--- a/src/components/Link/Link.tsx
+++ b/src/components/Link/Link.tsx
@@ -92,7 +92,8 @@ export const Link = React.forwardRef<HTMLElement, LinkProps>(function Link(
                 {...(extraProps as React.HTMLAttributes<HTMLSpanElement>)}
                 {...commonProps}
                 ref={ref as React.Ref<HTMLSpanElement>}
-                role="link"
+                // FIXME Href always should be string
+                // eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex
                 tabIndex={0}
             />
         );
diff --git a/src/components/Table/hoc/withTableSettings/TableColumnSetup/TableColumnSetup.tsx b/src/components/Table/hoc/withTableSettings/TableColumnSetup/TableColumnSetup.tsx
index 482a361418..10bbc43631 100644
--- a/src/components/Table/hoc/withTableSettings/TableColumnSetup/TableColumnSetup.tsx
+++ b/src/components/Table/hoc/withTableSettings/TableColumnSetup/TableColumnSetup.tsx
@@ -201,7 +201,7 @@ export const TableColumnSetup = (props: TableColumnSetupProps) => {
 
     return (
         <div className={b(null, className)}>
-            {/* It is used to handle clicks on the switcher control */}
+            {/* FIXME change to renderProp or provide component's context */}
             {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
             <div className={b('control')} ref={refControl} onClick={handleControlClick}>
                 {switcher || (
diff --git a/src/components/UserAvatar/UserAvatar.scss b/src/components/UserAvatar/UserAvatar.scss
index 5f97b5a75a..483bd10804 100644
--- a/src/components/UserAvatar/UserAvatar.scss
+++ b/src/components/UserAvatar/UserAvatar.scss
@@ -14,16 +14,6 @@ $block: '.#{variables.$ns}user-avatar';
     background-position: center;
     background-size: cover;
 
-    outline: none;
-
-    &:focus {
-        box-shadow: 0 0 0 2px var(--g-color-line-focus);
-    }
-
-    &:focus:not(:focus-visible) {
-        box-shadow: none;
-    }
-
     &_size {
         &_xs {
             width: 24px;
diff --git a/src/components/UserAvatar/UserAvatar.tsx b/src/components/UserAvatar/UserAvatar.tsx
index abc2dc7c0d..864c9d5cb2 100644
--- a/src/components/UserAvatar/UserAvatar.tsx
+++ b/src/components/UserAvatar/UserAvatar.tsx
@@ -1,7 +1,6 @@
 import React from 'react';
 
 import {block} from '../utils/cn';
-import {useActionHandlers} from '../utils/useActionHandlers';
 
 import {SIZES} from './constants';
 import type {UserAvatarSize} from './types';
@@ -35,18 +34,10 @@ export const UserAvatar = React.forwardRef<HTMLDivElement, UserAvatarProps>(
             setIsErrored(false);
         }, [imgUrl]);
 
-        const {onKeyDown} = useActionHandlers(onClick);
-
         return (
-            <div
-                role="button"
-                tabIndex={0}
-                onKeyDown={onKeyDown}
-                className={b({size}, className)}
-                title={title}
-                onClick={onClick}
-                ref={ref}
-            >
+            // OnClick deprecated, will be deleted
+            // eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions
+            <div className={b({size}, className)} title={title} onClick={onClick} ref={ref}>
                 <img
                     className={b('figure')}
                     width={SIZES[size]}
diff --git a/src/hooks/useOutsideClick/__tests__/Demo.tsx b/src/hooks/useOutsideClick/__tests__/Demo.tsx
index 6c09acf949..7f7f013071 100644
--- a/src/hooks/useOutsideClick/__tests__/Demo.tsx
+++ b/src/hooks/useOutsideClick/__tests__/Demo.tsx
@@ -1,4 +1,3 @@
-/* eslint-disable jsx-a11y/click-events-have-key-events */
 import React from 'react';
 
 import {useOutsideClick} from '../useOutsideClick';
@@ -23,9 +22,9 @@ export const Demo = () => {
     return (
         <div>
             <h1>{status}</h1>
-            <div ref={observerRef} onClick={handleClick}>
+            <button ref={observerRef} onClick={handleClick}>
                 {'Target'}
-            </div>
+            </button>
             <div>{'Outside'}</div>
         </div>
     );