Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Button): allow to pass any valid <button> or <a> HTML attributes #1594

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(Button): allow to pass any valid <button> or <a> HTML attributes #1594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7576c6
feat(Button): allow to pass all corresponding html props
amje May 19, 2024
45116a9
fix: change props spread order
amje May 20, 2024
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
4 changes: 1 addition & 3 deletions src/components/Alert/Alert.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,7 @@ export const Alert = (props: AlertProps) => {
view="flat"
className={bAlert('close-btn')}
onClick={onClose}
extraProps={{
'aria-label': i18n('label_close'),
}}
aria-label={i18n('label_close')}
>
<Icon
data={Xmark}
Expand Down
2 changes: 1 addition & 1 deletion src/components/Alert/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export interface AlertActionsProps {
items?: AlertAction[];
children?: React.ReactNode | React.ReactNode[];
}
export interface AlertActionProps extends ButtonProps {}
export type AlertActionProps = ButtonProps;
export interface AlertTitleProps {
className?: string;
text: string;
Expand Down
142 changes: 78 additions & 64 deletions src/components/Button/Button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';

import type {DOMProps, QAProps} from '../types';
import type {QAProps} from '../types';
import {block} from '../utils/cn';
import {isIcon} from '../utils/common';
import {eventBroker} from '../utils/event-broker';
Expand Down Expand Up @@ -53,7 +53,7 @@

export type ButtonWidth = 'auto' | 'max';

export interface ButtonProps extends DOMProps, QAProps {
interface ButtonBaseProps extends QAProps {
/** Button appearance */
view?: ButtonView;
size?: ButtonSize;
Expand All @@ -62,60 +62,57 @@
disabled?: boolean;
loading?: boolean;
width?: ButtonWidth;
title?: string;
tabIndex?: number;
id?: string;
type?: 'button' | 'submit' | 'reset';
component?: React.ElementType;
href?: string;
target?: string;
rel?: string;
/**
* @deprecated
*/
extraProps?:
| React.ButtonHTMLAttributes<HTMLButtonElement>
| React.AnchorHTMLAttributes<HTMLAnchorElement>;
onClick?: React.MouseEventHandler<HTMLButtonElement | HTMLAnchorElement>;
onMouseEnter?: React.MouseEventHandler<HTMLButtonElement | HTMLAnchorElement>;
onMouseLeave?: React.MouseEventHandler<HTMLButtonElement | HTMLAnchorElement>;
onFocus?: React.FocusEventHandler<HTMLButtonElement | HTMLAnchorElement>;
onBlur?: React.FocusEventHandler<HTMLButtonElement | HTMLAnchorElement>;
/** Button content. You can mix button text with `<Icon/>` component */
children?: React.ReactNode;
}

type AnchorOnlyHTMLAttributes = Omit<
React.AnchorHTMLAttributes<HTMLAnchorElement>,
keyof React.ButtonHTMLAttributes<HTMLButtonElement>
>;

export type ButtonButtonProps = ButtonBaseProps &
React.ButtonHTMLAttributes<HTMLButtonElement> & {
[K in keyof AnchorOnlyHTMLAttributes]: never;
} & {
type?: React.ButtonHTMLAttributes<HTMLButtonElement>['type'];
};

export type ButtonLinkProps = ButtonBaseProps &
React.AnchorHTMLAttributes<HTMLAnchorElement> & {
href: string;
};

export type ButtonProps = ButtonButtonProps | ButtonLinkProps;

const b = block('button');

const ButtonWithHandlers = React.forwardRef<HTMLElement, ButtonProps>(function Button(
{
const ButtonWithHandlers = React.forwardRef<HTMLElement, ButtonProps>(function Button(props, ref) {
const {
view = 'normal',
size = 'm',
pin = 'round-round',
selected,
disabled = false,
disabled: disabledProp = false,
loading = false,
width,
title,
tabIndex,
type = 'button',
component,
href,
target,
rel,
extraProps,
onClick,
onMouseEnter,
onMouseLeave,
onFocus,
onBlur,
children,
id,
style,
className,
qa,
},
ref,
) {
onClickCapture,
className,
component,
extraProps,
...restProps
} = props;
const handleClickCapture = React.useCallback(
(event: React.SyntheticEvent) => {
(event: React.MouseEvent) => {
eventBroker.publish({
componentId: 'Button',
eventId: 'click',
Expand All @@ -125,64 +122,81 @@
view,
},
});

if (onClickCapture) {
onClickCapture(event as any);

Check warning on line 127 in src/components/Button/Button.tsx

View workflow job for this annotation

GitHub Actions / Verify Files 

Unexpected any. Specify a different type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is type cast used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It requires weird type, intersection of HTMLButtonElement and HTMLAnchorElement

}
},
[view],
[view, onClickCapture],
);

const disabled = disabledProp || loading;
const commonProps = {
title,
tabIndex,
onClick,
onClickCapture: handleClickCapture,
onMouseEnter,
onMouseLeave,
onFocus,
onBlur,
id,
style,
className: b(
{
view,
size,
pin,
selected,
disabled: disabled || loading,
disabled,
loading,
width,
},
className,
),
'data-qa': qa,
};
const content = prepareChildren(children);

if (typeof href === 'string' || component) {
const linkProps = {
href,
target,
rel: target === '_blank' && !rel ? 'noopener noreferrer' : rel,
};
if (component) {
return React.createElement(
component || 'a',
component,
{
...extraProps,
...commonProps,
...(component ? {} : linkProps),
ref: ref as React.Ref<HTMLAnchorElement>,
'aria-disabled': disabled || loading,
ref,
tabIndex: disabled ? undefined : 0,
role: 'button',
'aria-disabled': disabled,
'aria-pressed': selected,
...restProps,
...extraProps,
},
prepareChildren(children),
content,
);
} else if (restProps.href) {
const linkProps = restProps as ButtonLinkProps;

return (
<a
{...commonProps}
ref={ref as React.Ref<HTMLAnchorElement>}
rel={
linkProps.target === '_blank' && !linkProps.rel
? 'noopener noreferrer'
: linkProps.rel
}
aria-disabled={disabled}
{...linkProps}
{...(extraProps as React.AnchorHTMLAttributes<HTMLAnchorElement>)}
>
{content}
</a>
);
} else {
const buttonProps = restProps as ButtonButtonProps;

return (
<button
{...(extraProps as React.ButtonHTMLAttributes<HTMLButtonElement>)}
{...commonProps}
ref={ref as React.Ref<HTMLButtonElement>}
type={type}
disabled={disabled || loading}
type={buttonProps.type ?? 'button'}
disabled={disabled}
aria-pressed={selected}
{...buttonProps}
{...(extraProps as React.ButtonHTMLAttributes<HTMLButtonElement>)}
>
{prepareChildren(children)}
{content}
</button>
);
}
Expand Down
42 changes: 15 additions & 27 deletions src/components/Button/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,33 +449,21 @@ LANDING_BLOCK-->

## Properties

| Name | Description | Type | Default |
| :----------- | :-------------------------------------------------------- | :-----------------------------: | :-------------: |
| children | Button content. You can mix text with `<Icon/>` component | `ReactNode` | |
| className | HTML `class` attribute | `string` | |
| component | Overrides the root component | `ElementType<any>` | `"button"` |
| disabled | Toggles `disabled` state | `false` | `false` |
| extraProps | Any additional props | `Record` | |
| href | HTML `href` attribute | `string` | |
| id | HTML `id` attribute | `string` | |
| loading | Toggles `loading` state | `false` | `false` |
| onBlur | `blur` event handler | `Function` | |
| onClick | `click` event handler | `Function` | |
| onFocus | `focus` event handler | `Function` | |
| onMouseEnter | `mouseenter` event handler | `Function` | |
| onMouseLeave | `mouseleave` event handler | `Function` | |
| pin | Sets button edges style | `string` | `"round-round"` |
| qa | HTML `data-qa` attribute, used in tests | `string` | |
| rel | HTML `rel` attribute | `string` | |
| selected | Toggles `selected` state | | |
| size | Sets button size | `string` | `"m"` |
| style | HTML `style` attribute | `React.CSSProperties` | |
| tabIndex | HTML `tabIndex` attribute | `number` | |
| target | HTML `target` attribute | `string` | |
| title | HTML `title` attribute | `string` | |
| type | HTML `type` attribute | `"button"` `"submit"` `"reset"` | `"button"` |
| view | Sets button appearance | `string` | `"normal"` |
| width | `"auto"` `"max"` | `"auto"` `"max"` | |
`ButtonProps` extends `React.ButtonHTMLAttributes` or `React.AnchorHTMLAttributes` based on passed `href` prop.

| Name | Description | Type | Default |
| :-------- | :-------------------------------------------------------- | :----------------: | :-------------: |
| children | Button content. You can mix text with `<Icon/>` component | `ReactNode` | |
| component | Overrides the root component | `ElementType<any>` | `"button"` |
| disabled | Toggles `disabled` state | `false` | `false` |
| href | HTML `href` attribute, forces to render an `<a>` element | `string` | |
| loading | Toggles `loading` state | `false` | `false` |
| pin | Sets button edges style | `string` | `"round-round"` |
| qa | HTML `data-qa` attribute, used in tests | `string` | |
| selected | Toggles `selected` state | | |
| size | Sets button size | `string` | `"m"` |
| view | Sets button appearance | `string` | `"normal"` |
| width | `"auto"` `"max"` | `"auto"` `"max"` | |

## CSS API

Expand Down
7 changes: 4 additions & 3 deletions src/components/Button/__stories__/Button.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {Meta, StoryObj} from '@storybook/react';

import {Showcase} from '../../../demo/Showcase';
import {Icon as IconComponent} from '../../Icon/Icon';
import type {ButtonLinkProps} from '../Button';
import {Button} from '../Button';

import {ButtonViewShowcase} from './ButtonViewShowcase';
Expand All @@ -37,7 +38,7 @@ export default {
},
},
},
} as Meta;
} as Meta<typeof Button>;

type Story = StoryObj<typeof Button>;

Expand Down Expand Up @@ -158,7 +159,7 @@ export const Pin: Story = {
},
};

export const Link: Story = {
export const Link: StoryObj<ButtonLinkProps> = {
args: {
...Default.args,
children: ['Link Button', <IconComponent key="icon" data={ArrowUpRightFromSquare} />],
Expand All @@ -180,7 +181,7 @@ export const InsideText: Story = {
<Button {...args} /> dolor
<br />
sit{' '}
<Button {...args} extraProps={{'aria-label': 'Icon button inside text'}}>
<Button {...args} aria-label="Icon button inside text">
<IconComponent data={Globe} />
</Button>{' '}
amet
Expand Down
8 changes: 4 additions & 4 deletions src/components/Button/__tests__/Button.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import userEvent from '@testing-library/user-event';

import {render, screen} from '../../../../test-utils/utils';
import {Button} from '../Button';
import type {ButtonPin, ButtonProps, ButtonSize, ButtonView} from '../Button';
import type {ButtonPin, ButtonSize, ButtonView} from '../Button';

const qaId = 'button-component';

Expand Down Expand Up @@ -105,11 +105,11 @@ describe('Button', () => {
test('should render custom component', () => {
const text = 'Button with custom component';

const ButtonComponent = (props: ButtonProps) => {
const ButtonComponent = (props: React.HTMLAttributes<HTMLElement>) => {
return (
<button {...props} style={{boxShadow: '2px 2px 2px 2px deepskyblue'}}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why element type changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To ensure any root tag is valid

<div {...props} style={{boxShadow: '2px 2px 2px 2px deepskyblue'}}>
{text}
</button>
</div>
);
};

Expand Down
15 changes: 3 additions & 12 deletions src/components/ClipboardButton/ClipboardButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';

import {ActionTooltip} from '../ActionTooltip';
import {Button} from '../Button';
import type {ButtonProps, ButtonSize} from '../Button';
import type {ButtonButtonProps, ButtonSize} from '../Button';
import {ClipboardIcon} from '../ClipboardIcon';
import {CopyToClipboard} from '../CopyToClipboard';
import type {CopyToClipboardProps, CopyToClipboardStatus} from '../CopyToClipboard/types';
Expand All @@ -14,7 +14,7 @@ export interface ClipboardButtonProps
Omit<ClipboardButtonComponentProps, 'status' | 'onClick'> {}

interface ClipboardButtonComponentProps
extends Omit<ButtonProps, 'href' | 'component' | 'target' | 'rel' | 'loading' | 'children'> {
extends Omit<ButtonButtonProps, 'component' | 'loading' | 'children' | 'onCopy'> {
status: CopyToClipboardStatus;
/** Disable tooltip. Tooltip won't be shown */
hasTooltip?: boolean;
Expand Down Expand Up @@ -42,7 +42,6 @@ const ClipboardButtonComponent = (props: ClipboardButtonComponentProps) => {
tooltipSuccessText = i18n('endCopy'),
status,
view = 'flat',
extraProps = {},
...rest
} = props;

Expand All @@ -51,15 +50,7 @@ const ClipboardButtonComponent = (props: ClipboardButtonComponentProps) => {
disabled={!hasTooltip}
title={status === 'success' ? tooltipSuccessText : tooltipInitialText}
>
<Button
view={view}
size={size}
extraProps={{
'aria-label': tooltipInitialText,
...extraProps,
}}
{...rest}
>
<Button view={view} size={size} aria-label={tooltipInitialText} {...rest}>
<Button.Icon>
<ClipboardIcon size={ButtonSizeToIconSize[size]} status={status} />
</Button.Icon>
Expand Down
Loading
Loading