Revert "feat(thumbnail): fix thumbnail avatar and userblock a11y"

gcornut · gcornut · commit 78cd32eb1218 · 2021-12-15T17:16:24.000+01:00
This reverts commit abbc4a2.
diff --git a/packages/lumx-core/src/scss/components/thumbnail/_index.scss b/packages/lumx-core/src/scss/components/thumbnail/_index.scss
@@ -103,9 +103,8 @@
 /* Thumbnail states
    ========================================================================== */
 
-.#{$lumx-base-prefix}-thumbnail--is-clickable {
+.#{$lumx-base-prefix}-thumbnail[tabindex='0'] {
     position: relative;
-    display: block;
     cursor: pointer;
 
     &::after {
@@ -120,8 +119,7 @@
         pointer-events: none;
     }
 
-    &:hover::after,
-    &:focus::after {
+    &:hover::after {
         @include lumx-state(lumx-base-const('state', 'HOVER'), lumx-base-const('emphasis', 'LOW'), 'dark');
     }
 
@@ -130,19 +128,19 @@
     }
 }
 
-.#{$lumx-base-prefix}-thumbnail--variant-rounded.#{$lumx-base-prefix}-thumbnail--is-clickable {
+.#{$lumx-base-prefix}-thumbnail--variant-rounded.#{$lumx-base-prefix}-thumbnail[tabindex='0'] {
     &[data-focus-visible-added]::after {
         border-radius: var(--lumx-border-radius);
     }
 }
 
-.#{$lumx-base-prefix}-thumbnail--theme-light.#{$lumx-base-prefix}-thumbnail--is-clickable {
+.#{$lumx-base-prefix}-thumbnail--theme-light.#{$lumx-base-prefix}-thumbnail[tabindex='0'] {
     &[data-focus-visible-added]::after {
         @include lumx-state(lumx-base-const('state', 'FOCUS'), lumx-base-const('emphasis', 'LOW'), 'dark');
     }
 }
 
-.#{$lumx-base-prefix}-thumbnail--theme-dark.#{$lumx-base-prefix}-thumbnail--is-clickable {
+.#{$lumx-base-prefix}-thumbnail--theme-dark.#{$lumx-base-prefix}-thumbnail[tabindex='0'] {
     &[data-focus-visible-added]::after {
         @include lumx-state(lumx-base-const('state', 'FOCUS'), lumx-base-const('emphasis', 'LOW'), 'light');
     }
diff --git a/packages/lumx-core/src/scss/components/user-block/_index.scss b/packages/lumx-core/src/scss/components/user-block/_index.scss
@@ -49,7 +49,7 @@
             color: lumx-color-variant('light', 'N');
         }
 
-        &--is-clickable {
+        &[tabindex='0'] {
             #{$self}--theme-light & {
                 @include lumx-link('dark');
             }
diff --git a/packages/lumx-react/src/components/avatar/Avatar.tsx b/packages/lumx-react/src/components/avatar/Avatar.tsx
@@ -36,10 +36,6 @@ export interface AvatarProps extends GenericProps {
         ThumbnailProps,
         'image' | 'alt' | 'size' | 'theme' | 'align' | 'fillHeight' | 'variant' | 'aspectRatio'
     >;
-    /** Props to pass to the link wrapping the thumbnail. */
-    linkProps?: React.DetailedHTMLProps<React.AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>;
-    /** Custom react component for the link (can be used to inject react router Link). */
-    linkAs?: 'a' | any;
 }
 
 /**
@@ -79,8 +75,6 @@ export const Avatar: Comp<AvatarProps, HTMLDivElement> = forwardRef((props, ref)
         size,
         theme,
         thumbnailProps,
-        linkProps,
-        linkAs,
         ...forwardedProps
     } = props;
 
@@ -91,8 +85,6 @@ export const Avatar: Comp<AvatarProps, HTMLDivElement> = forwardRef((props, ref)
             className={classNames(className, handleBasicClasses({ prefix: CLASSNAME, size, theme }))}
         >
             <Thumbnail
-                linkProps={linkProps}
-                linkAs={linkAs}
                 className={`${CLASSNAME}__thumbnail`}
                 onClick={onClick}
                 onKeyPress={onKeyPress}
diff --git a/packages/lumx-react/src/components/thumbnail/Thumbnail.stories.tsx b/packages/lumx-react/src/components/thumbnail/Thumbnail.stories.tsx
@@ -18,34 +18,13 @@ import { boolean, select, text } from '@storybook/addon-knobs';
 import { enumKnob } from '@lumx/react/stories/knobs/enumKnob';
 import { focusKnob } from '@lumx/react/stories/knobs/focusKnob';
 import { sizeKnob } from '@lumx/react/stories/knobs/sizeKnob';
-import classNames from 'classnames';
 
 export default { title: 'LumX components/thumbnail/Thumbnail' };
 
 export const Default = () => <Thumbnail alt="Image alt text" image={imageKnob()} size={Size.xxl} />;
 
 export const Clickable = () => <Thumbnail alt="Click me" image={imageKnob()} size={Size.xxl} onClick={console.log} />;
 
-export const ClickableLink = () => (
-    <Thumbnail alt="Click me" image={imageKnob()} size={Size.xxl} linkProps={{ href: 'https://google.fr' }} />
-);
-
-const CustomLinkComponent = (props: any) => (
-    <a {...props} className={classNames('custom-link-component', props.className)}>
-        {props.children}
-    </a>
-);
-
-export const ClickableCustomLink = () => (
-    <Thumbnail
-        alt="Click me"
-        image={imageKnob()}
-        size={Size.xxl}
-        linkAs={CustomLinkComponent}
-        linkProps={{ href: 'https://google.fr', className: 'custom-class-name' }}
-    />
-);
-
 export const DefaultFallback = () => <Thumbnail alt="foo" image="foo" />;
 
 export const IconFallback = () => <Thumbnail alt="foo" image="foo" fallback={mdiAbTesting} />;
diff --git a/packages/lumx-react/src/components/thumbnail/Thumbnail.test.tsx b/packages/lumx-react/src/components/thumbnail/Thumbnail.test.tsx
@@ -5,16 +5,7 @@ import 'jest-enzyme';
 import { commonTestsSuite, itShouldRenderStories } from '@lumx/react/testing/utils';
 
 import { Thumbnail, ThumbnailProps } from './Thumbnail';
-import {
-    Clickable,
-    ClickableCustomLink,
-    ClickableLink,
-    CustomFallback,
-    Default,
-    DefaultFallback,
-    IconFallback,
-    WithBadge,
-} from './Thumbnail.stories';
+import { Clickable, CustomFallback, Default, DefaultFallback, IconFallback, WithBadge } from './Thumbnail.stories';
 
 const CLASSNAME = Thumbnail.className as string;
 
@@ -31,16 +22,7 @@ describe(`<${Thumbnail.displayName}>`, () => {
     // 1. Test render via snapshot.
     describe('Snapshots and structure', () => {
         itShouldRenderStories(
-            {
-                Default,
-                Clickable,
-                ClickableLink,
-                ClickableCustomLink,
-                DefaultFallback,
-                CustomFallback,
-                IconFallback,
-                WithBadge,
-            },
+            { Default, Clickable, DefaultFallback, CustomFallback, IconFallback, WithBadge },
             Thumbnail,
         );
     });
diff --git a/packages/lumx-react/src/components/thumbnail/Thumbnail.tsx b/packages/lumx-react/src/components/thumbnail/Thumbnail.tsx
@@ -20,6 +20,7 @@ import { isInternetExplorer } from '@lumx/react/utils/isInternetExplorer';
 import { mergeRefs } from '@lumx/react/utils/mergeRefs';
 import { useFocusPoint } from '@lumx/react/components/thumbnail/useFocusPoint';
 import { useImageLoad } from '@lumx/react/components/thumbnail/useImageLoad';
+import { useClickable } from '@lumx/react/components/thumbnail/useClickable';
 import { FocusPoint, ThumbnailSize, ThumbnailVariant } from './types';
 
 type ImgHTMLProps = ImgHTMLAttributes<HTMLImageElement>;
@@ -62,10 +63,6 @@ export interface ThumbnailProps extends GenericProps {
     theme?: Theme;
     /** Variant of the component. */
     variant?: ThumbnailVariant;
-    /** Props to pass to the link wrapping the thumbnail. */
-    linkProps?: React.DetailedHTMLProps<React.AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>;
-    /** Custom react component for the link (can be used to inject react router Link). */
-    linkAs?: 'a' | any;
 }
 
 /**
@@ -112,8 +109,6 @@ export const Thumbnail: Comp<ThumbnailProps> = forwardRef((props, ref) => {
         size,
         theme,
         variant,
-        linkProps,
-        linkAs,
         ...forwardedProps
     } = props;
     const imgRef = useRef<HTMLImageElement>(null);
@@ -124,44 +119,24 @@ export const Thumbnail: Comp<ThumbnailProps> = forwardRef((props, ref) => {
     const isLoading = loadingState === 'isLoading';
 
     const [wrapper, setWrapper] = useState<HTMLElement>();
-
-    const isLink = Boolean(linkProps?.href || linkAs);
-    const isButton = !!forwardedProps.onClick;
-    const isClickable = isButton || isLink;
-
-    let Wrapper: any = 'div';
-    const wrapperProps = { ...forwardedProps };
-    if (isLink) {
-        Wrapper = linkAs || 'a';
-        Object.assign(wrapperProps, linkProps);
-    } else if (isButton) {
-        Wrapper = 'button';
-    }
+    const wrapperProps: any = {
+        ...forwardedProps,
+        ref: mergeRefs(setWrapper, ref),
+        className: classNames(
+            className,
+            handleBasicClasses({ align, aspectRatio, prefix: CLASSNAME, size, theme, variant, hasBadge: !!badge }),
+            isLoading && wrapper?.getBoundingClientRect()?.height && 'lumx-color-background-dark-L6',
+            fillHeight && `${CLASSNAME}--fill-height`,
+        ),
+        // Handle clickable Thumbnail a11y.
+        ...useClickable(props),
+    };
 
     // Update img style according to focus point and aspect ratio.
     const style = useFocusPoint({ image, focusPoint, aspectRatio, imgRef, loadingState, wrapper });
 
     return (
-        <Wrapper
-            {...wrapperProps}
-            ref={mergeRefs(setWrapper, ref) as any}
-            className={classNames(
-                linkProps?.className,
-                className,
-                handleBasicClasses({
-                    align,
-                    aspectRatio,
-                    prefix: CLASSNAME,
-                    size,
-                    theme,
-                    variant,
-                    isClickable,
-                    hasBadge: !!badge,
-                }),
-                isLoading && wrapper?.getBoundingClientRect()?.height && 'lumx-color-background-dark-L6',
-                fillHeight && `${CLASSNAME}--fill-height`,
-            )}
-        >
+        <div {...wrapperProps}>
             <div
                 className={`${CLASSNAME}__background`}
                 style={{
@@ -194,7 +169,7 @@ export const Thumbnail: Comp<ThumbnailProps> = forwardRef((props, ref) => {
                 ))}
             {badge &&
                 React.cloneElement(badge, { className: classNames(`${CLASSNAME}__badge`, badge.props.className) })}
-        </Wrapper>
+        </div>
     );
 });
 Thumbnail.displayName = COMPONENT_NAME;
diff --git a/packages/lumx-react/src/components/thumbnail/__snapshots__/Thumbnail.test.tsx.snap b/packages/lumx-react/src/components/thumbnail/__snapshots__/Thumbnail.test.tsx.snap
@@ -1,9 +1,12 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`<Thumbnail> Snapshots and structure should render story 'Clickable' 1`] = `
-<button
-  className="lumx-thumbnail lumx-thumbnail--size-xxl lumx-thumbnail--theme-light lumx-thumbnail--is-clickable"
+<div
+  className="lumx-thumbnail lumx-thumbnail--size-xxl lumx-thumbnail--theme-light"
   onClick={[Function]}
+  onKeyPress={[Function]}
+  role="button"
+  tabIndex={0}
 >
   <div
     className="lumx-thumbnail__background"
@@ -22,57 +25,7 @@ exports[`<Thumbnail> Snapshots and structure should render story 'Clickable' 1`]
       style={Object {}}
     />
   </div>
-</button>
-`;
-
-exports[`<Thumbnail> Snapshots and structure should render story 'ClickableCustomLink' 1`] = `
-<CustomLinkComponent
-  className="custom-class-name lumx-thumbnail lumx-thumbnail--size-xxl lumx-thumbnail--theme-light lumx-thumbnail--is-clickable"
-  href="https://google.fr"
->
-  <div
-    className="lumx-thumbnail__background"
-    style={
-      Object {
-        "display": undefined,
-        "visibility": "hidden",
-      }
-    }
-  >
-    <img
-      alt="Click me"
-      className="lumx-thumbnail__image"
-      loading="lazy"
-      src="/demo-assets/landscape1.jpg"
-      style={Object {}}
-    />
-  </div>
-</CustomLinkComponent>
-`;
-
-exports[`<Thumbnail> Snapshots and structure should render story 'ClickableLink' 1`] = `
-<a
-  className="lumx-thumbnail lumx-thumbnail--size-xxl lumx-thumbnail--theme-light lumx-thumbnail--is-clickable"
-  href="https://google.fr"
->
-  <div
-    className="lumx-thumbnail__background"
-    style={
-      Object {
-        "display": undefined,
-        "visibility": "hidden",
-      }
-    }
-  >
-    <img
-      alt="Click me"
-      className="lumx-thumbnail__image"
-      loading="lazy"
-      src="/demo-assets/landscape1.jpg"
-      style={Object {}}
-    />
-  </div>
-</a>
+</div>
 `;
 
 exports[`<Thumbnail> Snapshots and structure should render story 'CustomFallback' 1`] = `
diff --git a/packages/lumx-react/src/components/user-block/UserBlock.stories.tsx b/packages/lumx-react/src/components/user-block/UserBlock.stories.tsx
diff --git a/packages/lumx-react/src/components/user-block/UserBlock.tsx b/packages/lumx-react/src/components/user-block/UserBlock.tsx
diff --git a/packages/lumx-react/src/components/user-block/__snapshots__/UserBlock.test.tsx.snap b/packages/lumx-react/src/components/user-block/__snapshots__/UserBlock.test.tsx.snap

Original file line number	Diff line number	Diff line change
`@@ -103,9 +103,8 @@`
`103`	`103`	`/* Thumbnail states`
`104`	`104`	`========================================================================== */`
`105`	`105`
`106`		`-.#{$lumx-base-prefix}-thumbnail--is-clickable {`
	`106`	`+.#{$lumx-base-prefix}-thumbnail[tabindex='0'] {`
`107`	`107`	`position: relative;`
`108`		`- display: block;`
`109`	`108`	`cursor: pointer;`
`110`	`109`
`111`	`110`	`&::after {`
`@@ -120,8 +119,7 @@`
`120`	`119`	`pointer-events: none;`
`121`	`120`	`}`
`122`	`121`
`123`		`- &:hover::after,`
`124`		`- &:focus::after {`
	`122`	`+ &:hover::after {`
`125`	`123`	`@include lumx-state(lumx-base-const('state', 'HOVER'), lumx-base-const('emphasis', 'LOW'), 'dark');`
`126`	`124`	`}`
`127`	`125`
`@@ -130,19 +128,19 @@`
`130`	`128`	`}`
`131`	`129`	`}`
`132`	`130`
`133`		`-.#{$lumx-base-prefix}-thumbnail--variant-rounded.#{$lumx-base-prefix}-thumbnail--is-clickable {`
	`131`	`+.#{$lumx-base-prefix}-thumbnail--variant-rounded.#{$lumx-base-prefix}-thumbnail[tabindex='0'] {`
`134`	`132`	`&[data-focus-visible-added]::after {`
`135`	`133`	`border-radius: var(--lumx-border-radius);`
`136`	`134`	`}`
`137`	`135`	`}`
`138`	`136`
`139`		`-.#{$lumx-base-prefix}-thumbnail--theme-light.#{$lumx-base-prefix}-thumbnail--is-clickable {`
	`137`	`+.#{$lumx-base-prefix}-thumbnail--theme-light.#{$lumx-base-prefix}-thumbnail[tabindex='0'] {`
`140`	`138`	`&[data-focus-visible-added]::after {`
`141`	`139`	`@include lumx-state(lumx-base-const('state', 'FOCUS'), lumx-base-const('emphasis', 'LOW'), 'dark');`
`142`	`140`	`}`
`143`	`141`	`}`
`144`	`142`
`145`		`-.#{$lumx-base-prefix}-thumbnail--theme-dark.#{$lumx-base-prefix}-thumbnail--is-clickable {`
	`143`	`+.#{$lumx-base-prefix}-thumbnail--theme-dark.#{$lumx-base-prefix}-thumbnail[tabindex='0'] {`
`146`	`144`	`&[data-focus-visible-added]::after {`
`147`	`145`	`@include lumx-state(lumx-base-const('state', 'FOCUS'), lumx-base-const('emphasis', 'LOW'), 'light');`
`148`	`146`	`}`
Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@`
`49`	`49`	`color: lumx-color-variant('light', 'N');`
`50`	`50`	`}`
`51`	`51`
`52`		`- &--is-clickable {`
	`52`	`+ &[tabindex='0'] {`
`53`	`53`	`#{$self}--theme-light & {`
`54`	`54`	`@include lumx-link('dark');`
`55`	`55`	`}`