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.

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): consume Penta tokens and update examples #9934

Merged
merged 4 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
30 changes: 21 additions & 9 deletions packages/react-core/src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ export enum ButtonVariant {
warning = 'warning',
link = 'link',
plain = 'plain',
control = 'control'
control = 'control',
stateful = 'stateful'
}

export enum ButtonType {
Expand All @@ -28,6 +29,12 @@ export enum ButtonSize {
lg = 'lg'
}

export enum ButtonState {
read = 'read',
unread = 'unread',
attention = 'attention'
}

export interface BadgeCountObject {
/** Adds styling to the badge to indicate it has been read */
isRead?: boolean;
Expand All @@ -44,8 +51,8 @@ export interface ButtonProps extends Omit<React.HTMLProps<HTMLButtonElement>, 'r
className?: string;
/** Sets the base component to render. defaults to button */
component?: React.ElementType<any> | React.ComponentType<any>;
/** Adds active styling to button. */
isActive?: boolean;
/** Adds clicked styling to button. */
isClicked?: boolean;
/** Adds block styling to button */
isBlock?: boolean;
/** Adds disabled styling and disables the button using the disabled html attribute */
Expand All @@ -69,7 +76,11 @@ export interface ButtonProps extends Omit<React.HTMLProps<HTMLButtonElement>, 'r
/** Sets button type */
type?: 'button' | 'submit' | 'reset';
/** Adds button variant styles */
variant?: 'primary' | 'secondary' | 'tertiary' | 'danger' | 'warning' | 'link' | 'plain' | 'control';
variant?: 'primary' | 'secondary' | 'tertiary' | 'danger' | 'warning' | 'link' | 'plain' | 'control' | 'stateful';
/** Sets state of the stateful button variant. Default is "unread" */
state?: 'read' | 'unread' | 'attention';
/** Applies no padding on a plain button variant. Use when plain button is placed inline with text */
noPadding?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

adamviktora marked this conversation as resolved.
Show resolved Hide resolved
/** Sets position of the icon. Note: "left" and "right" are deprecated. Use "start" and "end" instead */
iconPosition?: 'start' | 'end' | 'left' | 'right';
/** Adds accessible text to the button. */
Expand All @@ -94,9 +105,7 @@ const ButtonBase: React.FunctionComponent<ButtonProps> = ({
children = null,
className = '',
component = 'button',
// TODO: Update eslint ignore when issue #9907 is resolved
// eslint-disable-next-line @typescript-eslint/no-unused-vars
isActive = false,
isClicked = false,
isBlock = false,
isDisabled = false,
isAriaDisabled = false,
Expand All @@ -110,6 +119,8 @@ const ButtonBase: React.FunctionComponent<ButtonProps> = ({
isInline = false,
type = ButtonType.button,
variant = ButtonVariant.primary,
state = ButtonState.unread,
noPadding = false,
iconPosition = 'start',
'aria-label': ariaLabel = null,
icon = null,
Expand Down Expand Up @@ -157,12 +168,13 @@ const ButtonBase: React.FunctionComponent<ButtonProps> = ({
isBlock && styles.modifiers.block,
isDisabled && styles.modifiers.disabled,
isAriaDisabled && styles.modifiers.ariaDisabled,
// TODO: Update when issue #9907 is resolved
// isActive && styles.modifiers.active,
isClicked && styles.modifiers.clicked,
isInline && variant === ButtonVariant.link && styles.modifiers.inline,
isDanger && (variant === ButtonVariant.secondary || variant === ButtonVariant.link) && styles.modifiers.danger,
isLoading !== null && variant !== ButtonVariant.plain && styles.modifiers.progress,
isLoading && styles.modifiers.inProgress,
noPadding && variant === ButtonVariant.plain && styles.modifiers.noPadding,
variant === ButtonVariant.stateful && styles.modifiers[state],
size === ButtonSize.sm && styles.modifiers.small,
size === ButtonSize.lg && styles.modifiers.displayLg,
className
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';

import { render, screen } from '@testing-library/react';

import { Button, ButtonVariant } from '../Button';
import { Button, ButtonState, ButtonVariant } from '../Button';

Object.values(ButtonVariant).forEach((variant) => {
if (variant !== 'primary') {
Expand Down Expand Up @@ -55,10 +55,9 @@ test('Renders with class pf-m-block when isBlock = true', () => {
expect(screen.getByRole('button')).toHaveClass('pf-m-block');
});

// TODO: Reenable or remove with issue #9907
xtest('Renders with class pf-m-active when isActive = true', () => {
render(<Button isActive>Active Button</Button>);
expect(screen.getByRole('button')).toHaveClass('pf-m-active');
xtest('Renders with class pf-m-clicked when isClicked = true', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we reenable this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, there will no longer be the isActive prop and the pf-m-active modifier in V6, it is renamed to isClicked. So I don't think we should reenable that test.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like you modified it for the isClicked. Does the test fail? I that why it is still disabled?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have opened this issue #9989

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I did not realize that when it is marked as xtest and not test that it means the test is disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix PR: #10013

render(<Button isClicked>Clicked Button</Button>);
expect(screen.getByRole('button')).toHaveClass('pf-m-clicked');
});

test('Renders with class pf-m-disabled when isDisabled = true', () => {
Expand All @@ -80,6 +79,22 @@ test('Does not disable button when isDisabled = true and component = a', () => {
expect(screen.getByText('Disabled yet focusable button')).not.toHaveProperty('disabled');
});

test('Renders with class pf-m-unread by default when variant = stateful', () => {
render(<Button variant="stateful">Stateful Button</Button>);
expect(screen.getByRole('button')).toHaveClass('pf-m-stateful', 'pf-m-unread');
});

Object.values(ButtonState).forEach((state) => {
test(`Renders with class pf-m-${state} when state = ${state} and variant = stateful`, () => {
render(
<Button variant="stateful" state={state}>
Stateful Button - {state}
</Button>
);
expect(screen.getByRole('button')).toHaveClass('pf-m-stateful', `pf-m-${state}`);
});
});

test('Renders with class pf-m-danger when isDanger = true and variant = secondary', () => {
render(
<Button variant="secondary" isDanger>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ exports[`Renders basic button 1`] = `
aria-disabled="false"
aria-label="basic button"
class="pf-v5-c-button pf-m-primary"
data-ouia-component-id="OUIA-Generated-Button-primary-26"
data-ouia-component-id="OUIA-Generated-Button-primary-27"
data-ouia-component-type="PF5/Button"
data-ouia-safe="true"
type="button"
Expand Down
17 changes: 17 additions & 0 deletions packages/react-core/src/components/Button/examples/Button.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import ExternalLinkSquareAltIcon from '@patternfly/react-icons/dist/esm/icons/ex
import CopyIcon from '@patternfly/react-icons/dist/esm/icons/copy-icon';
import ArrowRightIcon from '@patternfly/react-icons/dist/esm/icons/arrow-right-icon';
import UploadIcon from '@patternfly/react-icons/dist/esm/icons/upload-icon';
import BellIcon from '@patternfly/react-icons/dist/esm/icons/bell-icon';
import QuestionCircleIcon from '@patternfly/react-icons/dist/esm/icons/question-circle-icon';
import { Link } from '@reach/router';

## Examples
Expand All @@ -33,6 +35,7 @@ PatternFly supports several button styling variants to be used in different scen
| Link | Links are labeled, but have no background or border. Use for actions that require less emphasis, actions that navigate users to another page within the same window, and/or actions that navigate to external pages in a new window. Links may be placed inline with text using the `isInline` property.|
| Plain | Plain buttons have no styling and are intended to be labeled with icons. |
| Control | Control buttons can be labeled with text or icons. Primarily intended to be paired with other controls in an [input group](/components/input-group). |
| Stateful | Stateful buttons are ideal for displaying the state of notifications. They have 3 states: `read`, `unread` and `attention`.

### Disabled buttons

Expand Down Expand Up @@ -107,3 +110,17 @@ Buttons can display a `count` in the form of a badge to indicate some value or n

```ts file="./ButtonWithCount.tsx" isBeta
```

### Plain with no padding

To display a plain/icon button inline with text, use `noPadding` prop in addition to `variant="plain"`.

```ts file="./ButtonPlainNoPadding.tsx"
```

### Stateful

Stateful buttons are ideal for displaying the state of notifications. Use `variant="stateful"` alongside with the `state` property, which can have these 3 values: `read`, `unread` (used as default) and `attention` (which means unread and requires attention).

```ts file="./ButtonStateful.tsx"
```
Original file line number Diff line number Diff line change
@@ -1,52 +1,28 @@
import React from 'react';
import { Button, Tooltip } from '@patternfly/react-core';
import TimesIcon from '@patternfly/react-icons/dist/esm/icons/times-icon';
import { Button, Flex, Tooltip } from '@patternfly/react-core';
import PlusCircleIcon from '@patternfly/react-icons/dist/esm/icons/plus-circle-icon';

export const ButtonAriaDisabled: React.FunctionComponent = () => (
<React.Fragment>
<Button isAriaDisabled>Primary aria disabled</Button>
<Button isAriaDisabled>Secondary aria disabled</Button>{' '}
<Button variant="secondary" isDanger isAriaDisabled>
Danger secondary aria disabled
</Button>{' '}
<Button isAriaDisabled variant="tertiary">
Tertiary aria disabled
</Button>{' '}
<Button isAriaDisabled variant="danger">
Danger disabled
</Button>{' '}
<Button isAriaDisabled variant="warning">
Warning disabled
</Button>
<br />
<br />
<Button isAriaDisabled variant="link" icon={<PlusCircleIcon />}>
Link aria disabled
</Button>{' '}
<Button isAriaDisabled variant="link" isInline>
Inline link aria disabled
</Button>{' '}
<Button variant="link" isDanger isAriaDisabled>
Danger link disabled
</Button>{' '}
<Button isAriaDisabled variant="plain" aria-label="Action">
<TimesIcon />
</Button>{' '}
<Button isAriaDisabled variant="control">
Control aria disabled
</Button>
<br />
<br />
<Tooltip content="Aria-disabled buttons are like disabled buttons, but focusable. Allows for tooltip support.">
<Button isAriaDisabled variant="secondary">
Secondary button to core docs
<>
adamviktora marked this conversation as resolved.
Show resolved Hide resolved
<Flex columnGap={{ default: 'columnGapSm' }}>
<Button isAriaDisabled>Primary aria disabled</Button>
<Button isAriaDisabled variant="link" icon={<PlusCircleIcon />}>
Link aria disabled
</Button>
</Tooltip>{' '}
<Tooltip content="Aria-disabled link as button with tooltip">
<Button component="a" isAriaDisabled href="https://pf4.patternfly.org/" target="_blank" variant="tertiary">
Tertiary link as button to core docs
<Button isAriaDisabled variant="link" isInline>
Inline link aria disabled
</Button>
</Tooltip>
</React.Fragment>
</Flex>
<br />
<Flex columnGap={{ default: 'columnGapSm' }}>
<Tooltip content="Aria-disabled buttons are like disabled buttons, but focusable. Allows for tooltip support.">
<Button isAriaDisabled>Primary button with tooltip</Button>
</Tooltip>
<Tooltip content="Aria-disabled link as button with tooltip">
<Button component="a" isAriaDisabled href="https://www.patternfly.org/" target="_blank" variant="secondary">
Secondary link as button to PatternFly home
</Button>
</Tooltip>
</Flex>
</>
);
Original file line number Diff line number Diff line change
@@ -1,22 +1,20 @@
import React from 'react';
import { Button } from '@patternfly/react-core';
import { Button, Flex } from '@patternfly/react-core';
import ArrowRightIcon from '@patternfly/react-icons/dist/esm/icons/arrow-right-icon';

export const ButtonCallToAction: React.FunctionComponent = () => (
<React.Fragment>
<Flex columnGap={{ default: 'columnGapSm' }}>
<Button variant="primary" size="lg">
Call to action
</Button>{' '}
</Button>
<Button variant="secondary" size="lg">
Call to action
</Button>{' '}
</Button>
<Button variant="tertiary" size="lg">
Call to action
</Button>{' '}
<Button variant="link" size="lg">
Call to action <ArrowRightIcon />
</Button>
<br />
<br />
</React.Fragment>
<Button variant="link" size="lg" icon={<ArrowRightIcon />} iconPosition="end">
Call to action
</Button>
</Flex>
);
Original file line number Diff line number Diff line change
@@ -1,39 +1,52 @@
import React from 'react';
import { Button } from '@patternfly/react-core';
import { Button, Flex } from '@patternfly/react-core';
import TimesIcon from '@patternfly/react-icons/dist/esm/icons/times-icon';
import PlusCircleIcon from '@patternfly/react-icons/dist/esm/icons/plus-circle-icon';
import CopyIcon from '@patternfly/react-icons/dist/esm/icons/copy-icon';

export const ButtonDisabled: React.FunctionComponent = () => (
<React.Fragment>
adamviktora marked this conversation as resolved.
Show resolved Hide resolved
<Button isDisabled>Primary disabled</Button> <Button isDisabled>Secondary disabled</Button>{' '}
<Button variant="secondary" isDanger isDisabled>
Danger secondary disabled
</Button>{' '}
<Button isDisabled variant="tertiary">
Tertiary disabled
</Button>{' '}
<Button isDisabled variant="danger">
Danger disabled
</Button>{' '}
<Button isDisabled variant="warning">
Warning disabled
</Button>
<Flex columnGap={{ default: 'columnGapSm' }}>
<Button isDisabled>Primary</Button>
<Button variant="secondary" isDisabled>
Secondary
</Button>
<Button variant="secondary" isDanger isDisabled>
Danger secondary
</Button>
<Button isDisabled variant="tertiary">
Tertiary
</Button>
<Button isDisabled variant="danger">
Danger
</Button>
<Button isDisabled variant="warning">
Warning
</Button>
</Flex>
<br />
<Flex columnGap={{ default: 'columnGapSm' }}>
<Button isDisabled variant="link" icon={<PlusCircleIcon />}>
Link
</Button>
<Button isDisabled variant="link" isInline>
Inline link
</Button>
<Button variant="link" isDanger isDisabled>
Danger link
</Button>
<Button isDisabled variant="plain" aria-label="Action">
<TimesIcon />
</Button>
</Flex>
<br />
<Button isDisabled variant="link" icon={<PlusCircleIcon />}>
Link disabled
</Button>{' '}
<Button isDisabled variant="link" isInline>
Inline link disabled
</Button>{' '}
<Button variant="link" isDanger isDisabled>
Danger link disabled
</Button>{' '}
<Button isDisabled variant="plain" aria-label="Action">
<TimesIcon />
</Button>{' '}
<Button isDisabled variant="control">
Control disabled
</Button>
<Flex columnGap={{ default: 'columnGapSm' }}>
<Button isDisabled variant="control">
Control
</Button>
<Button isDisabled variant="control" aria-label="Copy">
<CopyIcon />
</Button>
</Flex>
</React.Fragment>
);
28 changes: 14 additions & 14 deletions packages/react-core/src/components/Button/examples/ButtonLinks.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import React from 'react';
import { Button } from '@patternfly/react-core';
import { Button, Flex } from '@patternfly/react-core';

export const ButtonLinks: React.FunctionComponent = () => (
<React.Fragment>
<Button component="a" href="https://pf4.patternfly.org/" target="_blank" variant="primary">
Link to core docs
</Button>{' '}
<Button component="a" href="https://pf4.patternfly.org/" target="_blank" variant="secondary">
Secondary link to core docs
</Button>{' '}
<Button isDisabled component="a" href="https://pf4.patternfly.org/" target="_blank" variant="tertiary">
Tertiary link to core docs
</Button>{' '}
<Button component="a" href="https://pf4.patternfly.org/contribution/#modifiers" variant="link">
Jump to modifiers in contribution guidelines
<Flex>
<Button component="a" href="https://www.patternfly.org/" target="_blank" variant="primary">
Link to PatternFly home
</Button>
</React.Fragment>
<Button component="a" href="https://www.patternfly.org/" target="_blank" variant="secondary">
Secondary link to PatternFly home
</Button>
<Button isDisabled component="a" href="https://www.patternfly.org/" target="_blank" variant="tertiary">
Tertiary link to PatternFly home
</Button>
<Button component="a" href="https://www.patternfly.org/" variant="link">
Jump to PatternFly home
</Button>
</Flex>
);
Loading
Loading