refactor(components): uniformize handling of link and button

Author: gcornut

CHANGELOG.md

@@ -15,6 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 -   `@lumx/react`:
     -   Import types and constants from `@lumx/core` and re-export
     -   Import `className` utilities from `@lumx/core`
+    -   Uniformize link and button handling in `Link`, `Button`, `SideNavigationItem`, `Thumbnail`, `NavigationItem`
+        and `ListItem`.
+    -   Render disabled link as link instead of disabled buttons.
 
 ## [3.19.0][] - 2025-11-07
 

packages/lumx-react/src/components/button/Button.test.tsx

@@ -78,9 +78,10 @@ describe(`<${Button.displayName}>`, () => {
         it('should render disabled link', async () => {
             const onClick = jest.fn();
             const { button } = setup({ children: 'Label', disabled: true, href: 'https://example.com', onClick });
-            // Disabled link do not exist so we fallback to a button
-            expect(screen.queryByRole('link')).not.toBeInTheDocument();
-            expect(button).toHaveAttribute('disabled');
+            expect(screen.queryByRole('link')).toBeInTheDocument();
+            expect(button).toHaveAttribute('aria-disabled', 'true');
+            // Simulate standard disabled state (not focusable)
+            expect(button).toHaveAttribute('tabindex', '-1');
             await userEvent.click(button);
             expect(onClick).not.toHaveBeenCalled();
         });
@@ -102,8 +103,7 @@ describe(`<${Button.displayName}>`, () => {
                 onClick,
             });
             expect(button).toHaveAccessibleName('Label');
-            // Disabled link do not exist so we fallback to a button
-            expect(screen.queryByRole('link')).not.toBeInTheDocument();
+            expect(screen.queryByRole('link')).toBeInTheDocument();
             expect(button).toHaveAttribute('aria-disabled', 'true');
             await userEvent.click(button);
             expect(onClick).not.toHaveBeenCalled();

packages/lumx-react/src/components/button/ButtonRoot.tsx

@@ -1,17 +1,15 @@
 import React, { AriaAttributes, ButtonHTMLAttributes, DetailedHTMLProps, RefObject } from 'react';
 
-import isEmpty from 'lodash/isEmpty';
-
 import classNames from 'classnames';
 
 import { ColorPalette, Emphasis, Size, Theme } from '@lumx/react';
 import { CSS_PREFIX } from '@lumx/react/constants';
 import { GenericProps, HasTheme } from '@lumx/react/utils/type';
 import { handleBasicClasses } from '@lumx/core/js/utils/className';
-import { renderLink } from '@lumx/react/utils/react/renderLink';
 import { forwardRef } from '@lumx/react/utils/react/forwardRef';
-import { useDisableStateProps } from '@lumx/react/utils/disabled/useDisableStateProps';
 import { HasAriaDisabled } from '@lumx/react/utils/type/HasAriaDisabled';
+import { RawClickable } from '@lumx/react/utils/react/RawClickable';
+import { useDisableStateProps } from '@lumx/react/utils/disabled';
 
 type HTMLButtonProps = DetailedHTMLProps<ButtonHTMLAttributes<HTMLButtonElement>, HTMLButtonElement>;
 
@@ -107,18 +105,14 @@ export const ButtonRoot = forwardRef<ButtonRootProps, HTMLButtonElement | HTMLAn
         color,
         emphasis,
         hasBackground,
-        href,
         isSelected,
         isActive,
         isFocused,
         isHovered,
         linkAs,
-        name,
         size,
-        target,
         theme,
         variant,
-        type = 'button',
         fullWidth,
         ...forwardedProps
     } = otherProps;
@@ -139,7 +133,7 @@ export const ButtonRoot = forwardRef<ButtonRootProps, HTMLButtonElement | HTMLAn
             color: adaptedColor,
             emphasis,
             isSelected,
-            isDisabled: isAnyDisabled,
+            isDisabled: props.isDisabled || props['aria-disabled'],
             isActive,
             isFocused,
             isHovered,
@@ -151,42 +145,18 @@ export const ButtonRoot = forwardRef<ButtonRootProps, HTMLButtonElement | HTMLAn
         }),
     );
 
-    /**
-     * If the linkAs prop is used, we use the linkAs component instead of a <button>.
-     * If there is an href attribute, we display an <a> instead of a <button>.
-     *
-     * However, in any case, if the component is disabled, we returned a <button> since disabled is not compatible with <a>.
-     */
-    if ((linkAs || !isEmpty(props.href)) && !isAnyDisabled) {
-        return renderLink(
-            {
-                linkAs,
-                ...forwardedProps,
-                'aria-label': ariaLabel,
-                href,
-                target,
-                className: buttonClassName,
-                ref: ref as RefObject<HTMLAnchorElement>,
-            },
-            children,
-        );
-    }
     return (
-        <button
+        <RawClickable
+            as={linkAs || (forwardedProps.href ? 'a' : 'button')}
             {...forwardedProps}
             {...disabledStateProps}
             aria-disabled={isAnyDisabled}
             aria-label={ariaLabel}
             ref={ref as RefObject<HTMLButtonElement>}
             className={buttonClassName}
-            name={name}
-            type={
-                // eslint-disable-next-line react/button-has-type
-                type
-            }
         >
             {children}
-        </button>
+        </RawClickable>
     );
 });
 ButtonRoot.displayName = COMPONENT_NAME;

packages/lumx-react/src/components/link/Link.test.tsx

@@ -85,17 +85,20 @@ describe(`<${Link.displayName}>`, () => {
         it('should render disabled link', async () => {
             const onClick = jest.fn();
             const { link } = setup({ children: 'Label', isDisabled: true, href: 'https://example.com', onClick });
-            // Disabled link do not exist so we fallback to a button
-            expect(screen.queryByRole('link')).not.toBeInTheDocument();
-            expect(link).toHaveAttribute('disabled');
+            expect(screen.queryByRole('link')).toBeInTheDocument();
+            expect(link).toHaveAttribute('aria-disabled');
+            // Simulate standard disabled state (not focusable)
+            expect(link).toHaveAttribute('tabindex', '-1');
             await userEvent.click(link);
             expect(onClick).not.toHaveBeenCalled();
         });
 
         it('should render aria-disabled button', async () => {
             const onClick = jest.fn();
             const { link } = setup({ children: 'Label', 'aria-disabled': true, onClick });
-            expect(link).toHaveAttribute('aria-disabled');
+            expect(screen.queryByRole('button')).toBeInTheDocument();
+            expect(link).toHaveAttribute('aria-disabled', 'true');
+            expect(link).not.toHaveAttribute('tabindex');
             await userEvent.click(link);
             expect(onClick).not.toHaveBeenCalled();
         });
@@ -109,8 +112,7 @@ describe(`<${Link.displayName}>`, () => {
                 onClick,
             });
             expect(link).toHaveAccessibleName('Label');
-            // Disabled link do not exist so we fallback to a button
-            expect(screen.queryByRole('link')).not.toBeInTheDocument();
+            expect(screen.queryByRole('link')).toBeInTheDocument();
             expect(link).toHaveAttribute('aria-disabled', 'true');
             await userEvent.click(link);
             expect(onClick).not.toHaveBeenCalled();

packages/lumx-react/src/components/link/Link.tsx

@@ -12,8 +12,9 @@ import {
 } from '@lumx/core/js/utils/className';
 import { forwardRef } from '@lumx/react/utils/react/forwardRef';
 import { wrapChildrenIconWithSpaces } from '@lumx/react/utils/react/wrapChildrenIconWithSpaces';
-import { useDisableStateProps } from '@lumx/react/utils/disabled/useDisableStateProps';
 import { HasAriaDisabled } from '@lumx/react/utils/type/HasAriaDisabled';
+import { RawClickable } from '@lumx/react/utils/react/RawClickable';
+import { useDisableStateProps } from '@lumx/react/utils/disabled';
 
 type HTMLAnchorProps = React.DetailedHTMLProps<React.AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>;
 
@@ -67,38 +68,26 @@ const CLASSNAME = getRootClassName(COMPONENT_NAME);
  * @return React element.
  */
 export const Link = forwardRef<LinkProps, HTMLAnchorElement | HTMLButtonElement>((props, ref) => {
-    const { isAnyDisabled, disabledStateProps, otherProps } = useDisableStateProps(props);
+    const { disabledStateProps, otherProps } = useDisableStateProps(props);
     const {
         children,
         className,
         color: propColor,
         colorVariant: propColorVariant,
-        href,
         leftIcon,
-        linkAs,
         rightIcon,
-        target,
         typography,
+        linkAs,
         ...forwardedProps
     } = otherProps;
     const [color, colorVariant] = resolveColorWithVariants(propColor, propColorVariant);
 
-    const isLink = linkAs || href;
-    const Component = isLink && !isAnyDisabled ? linkAs || 'a' : 'button';
-    const baseProps: React.ComponentProps<typeof Component> = {};
-    if (Component === 'button') {
-        baseProps.type = 'button';
-        Object.assign(baseProps, disabledStateProps);
-    } else if (isLink) {
-        baseProps.href = href;
-        baseProps.target = target;
-    }
-
     return (
-        <Component
-            ref={ref}
+        <RawClickable
+            ref={ref as any}
+            as={linkAs || (forwardedProps.href ? 'a' : 'button')}
             {...forwardedProps}
-            {...baseProps}
+            {...disabledStateProps}
             className={classNames(
                 className,
                 handleBasicClasses({ prefix: CLASSNAME, color, colorVariant, hasTypography: !!typography }),
@@ -112,7 +101,7 @@ export const Link = forwardRef<LinkProps, HTMLAnchorElement | HTMLButtonElement>
                     {rightIcon && <Icon icon={rightIcon} className={`${CLASSNAME}__right-icon`} />}
                 </>,
             )}
-        </Component>
+        </RawClickable>
     );
 });
 Link.displayName = COMPONENT_NAME;

packages/lumx-react/src/components/list/ListItem.test.tsx

@@ -43,7 +43,7 @@ describe(`<${ListItem.displayName}>`, () => {
         it('should render disabled list item button', async () => {
             const onItemSelected = jest.fn();
             const { link } = setup({ children: 'Label', isDisabled: true, onItemSelected });
-            expect(link).toHaveAttribute('aria-disabled', 'true');
+            expect(link).toBeDisabled();
             // The `renderLink` util removes the onClick handler but `user-event` will also not fire events on disabled elements.
             if (link) await userEvent.click(link);
             expect(onItemSelected).not.toHaveBeenCalled();
@@ -57,7 +57,6 @@ describe(`<${ListItem.displayName}>`, () => {
                 linkProps: { href: 'https://example.com' },
                 onItemSelected,
             });
-            expect(link).not.toHaveAttribute('href');
             expect(link).toHaveAttribute('aria-disabled', 'true');
             if (link) await userEvent.click(link);
             expect(onItemSelected).not.toHaveBeenCalled();
@@ -79,7 +78,6 @@ describe(`<${ListItem.displayName}>`, () => {
                 linkProps: { href: 'https://example.com' },
                 onItemSelected,
             });
-            expect(link).not.toHaveAttribute('href');
             expect(link).toHaveAttribute('aria-disabled', 'true');
             if (link) await userEvent.click(link);
             expect(onItemSelected).not.toHaveBeenCalled();

packages/lumx-react/src/components/list/ListItem.tsx

@@ -1,16 +1,15 @@
-import React, { ReactNode, Ref, SyntheticEvent, useMemo } from 'react';
+import React, { ReactNode, Ref, SyntheticEvent } from 'react';
 
 import classNames from 'classnames';
 import isEmpty from 'lodash/isEmpty';
 
 import { ListProps, Size } from '@lumx/react';
 import { GenericProps } from '@lumx/react/utils/type';
-import { onEnterPressed, onButtonPressed } from '@lumx/core/js/utils';
 import { getRootClassName, handleBasicClasses } from '@lumx/core/js/utils/className';
-import { renderLink } from '@lumx/react/utils/react/renderLink';
 import { forwardRef } from '@lumx/react/utils/react/forwardRef';
 import { useDisableStateProps } from '@lumx/react/utils/disabled/useDisableStateProps';
 import { HasAriaDisabled } from '@lumx/react/utils/type/HasAriaDisabled';
+import { RawClickable } from '@lumx/react/utils/react/RawClickable';
 
 export type ListItemSize = Extract<Size, 'tiny' | 'regular' | 'big' | 'huge'>;
 
@@ -94,13 +93,6 @@ export const ListItem = forwardRef<ListItemProps, HTMLLIElement>((props, ref) =>
         ...forwardedProps
     } = otherProps;
 
-    const role = linkAs || linkProps.href ? 'link' : 'button';
-    const onKeyDown = useMemo(() => {
-        if (onItemSelected && role === 'link') return onEnterPressed(onItemSelected as any);
-        if (onItemSelected && role === 'button') return onButtonPressed(onItemSelected as any);
-        return undefined;
-    }, [role, onItemSelected]);
-
     const content = (
         <>
             {before && <div className={`${CLASSNAME}__before`}>{before}</div>}
@@ -123,28 +115,23 @@ export const ListItem = forwardRef<ListItemProps, HTMLLIElement>((props, ref) =>
         >
             {isClickable({ linkProps, onItemSelected }) ? (
                 /* Clickable list item */
-                renderLink(
-                    {
-                        linkAs,
-                        tabIndex: !disabledStateProps.disabled ? 0 : undefined,
-                        role,
-                        'aria-disabled': isAnyDisabled,
-                        ...linkProps,
-                        href: isAnyDisabled ? undefined : linkProps.href,
-                        className: classNames(
-                            handleBasicClasses({
-                                prefix: `${CLASSNAME}__link`,
-                                isHighlighted,
-                                isSelected,
-                                isDisabled: isAnyDisabled,
-                            }),
-                        ),
-                        onClick: isAnyDisabled ? undefined : onItemSelected,
-                        onKeyDown: isAnyDisabled ? undefined : onKeyDown,
-                        ref: linkRef,
-                    },
-                    content,
-                )
+                <RawClickable
+                    as={linkAs || (linkProps.href ? 'a' : 'button')}
+                    {...linkProps}
+                    {...disabledStateProps}
+                    className={classNames(
+                        handleBasicClasses({
+                            prefix: `${CLASSNAME}__link`,
+                            isHighlighted,
+                            isSelected,
+                            isDisabled: isAnyDisabled,
+                        }),
+                    )}
+                    onClick={onItemSelected}
+                    ref={linkRef}
+                >
+                    {content}
+                </RawClickable>
             ) : (
                 /* Non clickable list item */
                 <div className={`${CLASSNAME}__wrapper`}>{content}</div>

packages/lumx-react/src/components/navigation/NavigationItem.tsx

@@ -1,11 +1,12 @@
 import React, { ElementType, ReactNode } from 'react';
 import { Icon, Placement, Size, Tooltip, Text } from '@lumx/react';
 import { getRootClassName, handleBasicClasses } from '@lumx/core/js/utils/className';
-import { ComponentRef, HasClassName, HasPolymorphicAs, HasTheme } from '@lumx/react/utils/type';
+import { ComponentRef, HasClassName, HasPolymorphicAs, HasRequiredLinkHref, HasTheme } from '@lumx/react/utils/type';
 import classNames from 'classnames';
 import { forwardRefPolymorphic } from '@lumx/react/utils/react/forwardRefPolymorphic';
 import { useTheme } from '@lumx/react/utils/theme/ThemeContext';
 import { useOverflowTooltipLabel } from '@lumx/react/hooks/useOverflowTooltipLabel';
+import { RawClickable } from '@lumx/react/utils/react/RawClickable';
 
 type BaseNavigationItemProps = {
     /** Icon (SVG path). */
@@ -16,17 +17,14 @@ type BaseNavigationItemProps = {
     isCurrentPage?: boolean;
 };
 
-/** Make `href` required when `as` is `a` */
-type RequiredLinkHref<E> = E extends 'a' ? { href: string } : Record<string, unknown>;
-
 /**
  * Navigation item props
  */
 export type NavigationItemProps<E extends ElementType = 'a'> = HasPolymorphicAs<E> &
     HasTheme &
     HasClassName &
     BaseNavigationItemProps &
-    RequiredLinkHref<E>;
+    HasRequiredLinkHref<E>;
 
 /**
  * Component display name.
@@ -44,8 +42,6 @@ export const NavigationItem = Object.assign(
         const theme = useTheme();
         const { tooltipLabel, labelRef } = useOverflowTooltipLabel(label);
 
-        const buttonProps = Element === 'button' ? { type: 'button' } : {};
-
         return (
             <li
                 className={classNames(
@@ -57,14 +53,14 @@ export const NavigationItem = Object.assign(
                 )}
             >
                 <Tooltip label={tooltipLabel} placement={Placement.TOP}>
-                    <Element
+                    <RawClickable
+                        as={Element}
                         className={handleBasicClasses({
                             prefix: `${CLASSNAME}__link`,
                             isSelected: isCurrentPage,
                         })}
                         ref={ref as React.Ref<any>}
                         aria-current={isCurrentPage ? 'page' : undefined}
-                        {...buttonProps}
                         {...forwardedProps}
                     >
                         {icon ? (
@@ -74,7 +70,7 @@ export const NavigationItem = Object.assign(
                         <Text as="span" truncate className={`${CLASSNAME}__label`} ref={labelRef}>
                             {label}
                         </Text>
-                    </Element>
+                    </RawClickable>
                 </Tooltip>
             </li>
         );