Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Weird that these screenshot file names end with a hyphen, but I don't think it's related to anything in this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Maybe we open new issue?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

#2329 done!

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added e2e/screenshots/tag-button-size-small-.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
22 changes: 22 additions & 0 deletions packages/react/src/components/Button/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,28 @@ test('should render button as tag', () => {
expect(TagButton).toHaveClass('Tag');
});

test('should render button as small tag', () => {
render(
<Button variant="tag" size="small">
small tag
</Button>
);
const TagButton = screen.getByRole('button', { name: 'small tag' });
expect(TagButton).toHaveClass('Tag');
expect(TagButton).toHaveClass('Tag--small');
});

test('should not apply Tag--small class to non-tag variants', () => {
render(
// @ts-expect-error size is not valid for non-tag variants
<Button variant="primary" size="small">
primary
</Button>
);
const button = screen.getByRole('button', { name: 'primary' });
expect(button).not.toHaveClass('Tag--small');
});

test('should render button as badge', () => {
render(<Button variant="badge">badge</Button>);
const BadgeButton = screen.getByRole('button', { name: 'badge' });
Expand Down
19 changes: 16 additions & 3 deletions packages/react/src/components/Button/index.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
import React, { type ButtonHTMLAttributes, forwardRef, type Ref } from 'react';
import classNames from 'classnames';
import type { TagSize } from '../Tag';

export interface ButtonProps extends ButtonHTMLAttributes<HTMLButtonElement> {
interface ButtonBaseProps extends ButtonHTMLAttributes<HTMLButtonElement> {
buttonRef?: Ref<HTMLButtonElement>;
thin?: boolean;
}

interface ButtonTagProps extends ButtonBaseProps {
variant: 'tag';
size?: TagSize;
}

interface ButtonNonTagProps extends ButtonBaseProps {
variant?:
| 'primary'
| 'secondary'
Expand All @@ -11,16 +21,18 @@ export interface ButtonProps extends ButtonHTMLAttributes<HTMLButtonElement> {
| 'danger'
| 'danger-secondary'
| 'link'
| 'tag'
| 'badge';
thin?: boolean;
size?: never;
}

export type ButtonProps = ButtonTagProps | ButtonNonTagProps;
Comment thread
Bracciata marked this conversation as resolved.

const Button = forwardRef<HTMLButtonElement, ButtonProps>(
(
{
variant = 'primary',
thin,
size = 'default',
Comment thread
Bracciata marked this conversation as resolved.
Outdated
children,
className,
buttonRef,
Expand All @@ -40,6 +52,7 @@ const Button = forwardRef<HTMLButtonElement, ButtonProps>(
Link: variant === 'link',
Tag: variant === 'tag',
'Button--tag': variant === 'tag',
'Tag--small': variant === 'tag' && size === 'small',
'Button--thin': thin,
'Button--badge': variant === 'badge'
})}
Expand Down
36 changes: 36 additions & 0 deletions packages/react/src/components/Button/screenshots.e2e.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,42 @@ test('should have screenshot for Button[variant="tag"]', async ({
await expect(component).toHaveScreenshot('dark--button[variant=tag]');
});

test('should have screenshot for Button[variant="tag"][size="small"]', async ({
mount,
page
}) => {
const component = await mount(
<div>
<Button variant="tag" size="small">
Tag
</Button>
<Button variant="tag" size="small">
Hover
</Button>
<Button variant="tag" size="small">
Active
</Button>
<Button variant="tag" size="small">
Focus
</Button>
<Button variant="tag" size="small" disabled>
{' '}
Disabled
</Button>
</div>
);

await component.getByText('Hover').hover();
Comment thread
Bracciata marked this conversation as resolved.
Outdated
setActive(component.getByText('Active'));
await component.getByText('Focus').focus();
Comment on lines +595 to +596
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: there are some other .getByText calls that could be updated to .getByRole.


await expect(component).toHaveScreenshot('button[variant=tag][size=small]');
await setTheme(page, 'dark');
await expect(component).toHaveScreenshot(
'dark--button[variant=tag][size=small]'
);
});

test('should have screenshot for Button[variant="badge"]', async ({
mount,
page
Expand Down
6 changes: 4 additions & 2 deletions packages/react/src/components/CopyButton/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ import copyTextToClipboard from '../../utils/copyTextToClipboard';

type ButtonProps = React.ComponentProps<typeof Button>;

export interface CopyButtonProps
extends Omit<ButtonProps, 'onCopy' | 'onClick'> {
export interface CopyButtonProps extends Omit<
ButtonProps,
'onCopy' | 'onClick' | 'size'
> {
value: string;
variant?: Extract<
ButtonProps['variant'],
Expand Down
4 changes: 3 additions & 1 deletion packages/react/src/components/Tag/index.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import React from 'react';
import classNames from 'classnames';

export type TagSize = 'default' | 'small';

interface TagProps {
children: React.ReactNode;
className?: string;
size?: 'default' | 'small';
size?: TagSize;
}

export const TagLabel = ({ children, className, ...other }: TagProps) => (
Expand Down
6 changes: 6 additions & 0 deletions packages/react/src/components/TagButton/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ test('should render an icon in the button', () => {
);
});

test('should support size prop', () => {
renderDefaultTagButton({ size: 'small' });

expect(screen.getByRole('button')).toHaveClass('Tag--small');
});

test('returns no axe violations', async () => {
const { container } = renderDefaultTagButton();

Expand Down
13 changes: 9 additions & 4 deletions packages/react/src/components/TagButton/index.tsx
Original file line number Diff line number Diff line change
@@ -1,26 +1,31 @@
import React, { Ref } from 'react';
import React, { type ButtonHTMLAttributes, Ref } from 'react';
import Icon, { IconType } from '../Icon';
import { ContentNode } from '../../types';
import { TagLabel } from '../Tag';
import { TagLabel, type TagSize } from '../Tag';
import classNames from 'classnames';
import Button from '../Button';

interface TagButtonProps extends React.HTMLAttributes<HTMLButtonElement> {
interface TagButtonProps extends Omit<
ButtonHTMLAttributes<HTMLButtonElement>,
'value'
> {
label: ContentNode;
value: ContentNode;
icon: IconType;
onClick: (e: React.MouseEvent<HTMLButtonElement>) => void;
Comment thread
Bracciata marked this conversation as resolved.
size?: TagSize;
}

const TagButton = React.forwardRef(
(
{ label, value, icon, className, ...rest }: TagButtonProps,
{ label, value, icon, className, size, ...rest }: TagButtonProps,
ref: Ref<HTMLButtonElement>
) => {
return (
<Button
variant="tag"
className={classNames('TagButton', className)}
size={size}
ref={ref}
{...rest}
>
Expand Down
61 changes: 61 additions & 0 deletions packages/react/src/components/TagButton/screenshots.e2e.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import React from 'react';
import { test, expect } from '../../../../../e2e/screenshots';
import { setActive, setTheme } from '../../../../../e2e/helpers/playwright';
import { TagButton } from '../../../';

// eslint-disable-next-line @typescript-eslint/no-empty-function
const noop = () => {};

test('should have screenshot for TagButton[size="small"]', async ({
mount,
page
}) => {
const component = await mount(
<div>
<TagButton
label="Label:"
value="Value"
icon="pencil"
size="small"
onClick={noop}
/>
<TagButton
label="Label:"
value="Hover"
icon="pencil"
size="small"
onClick={noop}
/>
<TagButton
label="Label:"
value="Active"
icon="pencil"
size="small"
onClick={noop}
/>
<TagButton
label="Label:"
value="Focus"
icon="pencil"
size="small"
onClick={noop}
/>
<TagButton
label="Label:"
value="Disabled"
icon="pencil"
size="small"
disabled
onClick={noop}
/>
</div>
);

await component.getByText('Hover').hover();
setActive(component.getByText('Active'));
await component.getByText('Focus').focus();
Comment on lines +54 to +56
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: more .getByText calls that could be updated to .getByRole.


await expect(component).toHaveScreenshot('tag-button[size=small]');
await setTheme(page, 'dark');
await expect(component).toHaveScreenshot('dark--tag-button[size=small]');
});
2 changes: 1 addition & 1 deletion packages/react/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export { default as Sidebar, SideBarItem } from './components/SideBar';
export { default as Code } from './components/Code';
export { default as LoaderOverlay } from './components/LoaderOverlay';
export { default as Line } from './components/Line';
export { default as Tag, TagLabel } from './components/Tag';
export { default as Tag, TagLabel, type TagSize } from './components/Tag';
export { default as Badge, BadgeLabel } from './components/Badge';
export { default as ImpactBadge } from './components/ImpactBadge';
export { default as TagButton } from './components/TagButton';
Expand Down
Loading