Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Import types and constants from `@lumx/core` and re-export
- Import `className` utilities from `@lumx/core`
- Migrate tests to `vitest`
- Uniformize link and button handling in `Link`, `Button`, `SideNavigationItem`, `Thumbnail` and `NavigationItem`.
- Render disabled links as link instead of disabled buttons.

## [3.19.0][] - 2025-11-07

Expand Down
10 changes: 5 additions & 5 deletions packages/lumx-react/src/components/button/Button.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,10 @@ describe(`<${Button.displayName}>`, () => {
it('should render disabled link', async () => {
const onClick = vi.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();
});
Expand All @@ -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();
Expand Down
42 changes: 6 additions & 36 deletions packages/lumx-react/src/components/button/ButtonRoot.tsx
Original file line number Diff line number Diff line change
@@ -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>;

Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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;
Expand Down
14 changes: 8 additions & 6 deletions packages/lumx-react/src/components/link/Link.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,20 @@ describe(`<${Link.displayName}>`, () => {
it('should render disabled link', async () => {
const onClick = vi.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 = vi.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();
});
Expand All @@ -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();
Expand Down
29 changes: 9 additions & 20 deletions packages/lumx-react/src/components/link/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>;

Expand Down Expand Up @@ -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 }),
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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). */
Expand All @@ -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.
Expand All @@ -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(
Expand All @@ -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 ? (
Expand All @@ -74,7 +70,7 @@ export const NavigationItem = Object.assign(
<Text as="span" truncate className={`${CLASSNAME}__label`} ref={labelRef}>
{label}
</Text>
</Element>
</RawClickable>
</Tooltip>
</li>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { ThemeProvider, useTheme } from '@lumx/react/utils/theme/ThemeContext';
import { useId } from '@lumx/react/hooks/useId';
import { forwardRef } from '@lumx/react/utils/react/forwardRef';

import { RawClickable } from '@lumx/react/utils/react/RawClickable';
import { CLASSNAME as ITEM_CLASSNAME } from './NavigationItem';
import { NavigationContext } from './context';

Expand Down Expand Up @@ -52,7 +53,8 @@ export const NavigationSection = forwardRef<NavigationSectionProps, HTMLLIElemen
)}
ref={ref}
>
<button
<RawClickable<'button'>
as="button"
{...forwardedProps}
aria-controls={sectionId}
aria-expanded={isOpen}
Expand All @@ -62,7 +64,6 @@ export const NavigationSection = forwardRef<NavigationSectionProps, HTMLLIElemen
setIsOpen(!isOpen);
event.stopPropagation();
}}
type="button"
>
{icon ? <Icon className={`${ITEM_CLASSNAME}__icon`} icon={icon} size={Size.xs} /> : null}

Expand All @@ -73,7 +74,7 @@ export const NavigationSection = forwardRef<NavigationSectionProps, HTMLLIElemen
className={classNames(`${ITEM_CLASSNAME}__icon`, `${CLASSNAME}__chevron`)}
icon={isOpen ? mdiChevronUp : mdiChevronDown}
/>
</button>
</RawClickable>
{isOpen &&
(isDropdown ? (
<Popover
Expand Down
Loading
Loading