Skip to content

Fix NotAuthorized default actions and clean the code #100

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

Merged
jessiehuff merged 10 commits into from
Feb 5, 2024
Merged

Fix NotAuthorized default actions and clean the code #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d6b933a
Fix NotAuthorized variant
fhlavac Jan 31, 2024
a0c0b6e
Clean CloseButton
fhlavac Jan 31, 2024
7394672
fix(docs): Update docs with links to original components
fhlavac Jan 31, 2024
558d525
Update snapshots
fhlavac Jan 31, 2024
cd36e9e
Update packages/module/patternfly-docs/content/extensions/component-g…
fhlavac Feb 1, 2024
325e09a
Update packages/module/patternfly-docs/content/extensions/component-g…
fhlavac Feb 1, 2024
8f150df
Update packages/module/patternfly-docs/content/extensions/component-g…
fhlavac Feb 1, 2024
c4c0bea
Fix wording and change confirmButtonVariant type
fhlavac Feb 1, 2024
f935fb7
Update CloseButton float value
fhlavac Feb 1, 2024
a3f8016
Update CloseButton padding
fhlavac Feb 2, 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
8 changes: 4 additions & 4 deletions ...ly-docs/content/extensions/component-groups/examples/CloseButton/CloseButton.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ propComponents: ['CloseButton']
import { CloseIcon } from '@patternfly/react-icons';
import CloseButton from '@patternfly/react-component-groups/dist/dynamic/CloseButton';

**Close button**
The **close button** component provides a way for users to exit a modal, dialogue, or similar action. To further customize this component, you can also utilize all properties of the [button component](/components/button).

The close button component provides a way for users to exit a modal, dialogue, or similar action.
## Examples

## Examples ##
### Basic close button

You can use onClick to execute a callback when a user selects the close button.
You can use `onClick` to execute a callback when a user clicks the close button.

```js file="./CloseButtonExample.tsx"

Expand Down
2 changes: 1 addition & 1 deletion ...nfly-docs/content/extensions/component-groups/examples/CloseButton/CloseButtonExample.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ import CloseButton from '@patternfly/react-component-groups/dist/dynamic/CloseBu

export const BasicExample: React.FunctionComponent = () => (
<>
<CloseButton onClick={()=>{console.log('Close button clicked')}} />
<CloseButton onClick={()=>{console.log('Close button clicked')}} style={{ float: 'none' }} />
</>
);
4 changes: 2 additions & 2 deletions ...nfly-docs/content/extensions/component-groups/examples/ErrorState/ErrorState.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ sourceLink: https://github.com/patternfly/react-component-groups/blob/main/packa

import ErrorState from "@patternfly/react-component-groups/dist/dynamic/ErrorState";

The **error state** component repurposes the `EmptyState` component to display an error to users.
The **error state** component repurposes the `EmptyState` component to display an error to users. To further customize this component, you can also utilize all properties of the [empty state component](/components/empty-state), with the `exception` of `children`.

## Examples

Expand All @@ -30,7 +30,7 @@ To provide users with error details, a basic error state should contain an appro

### Custom footer

To override default action button, specify your own using `customFooter`.
To override the default action button, specify your own using `customFooter`.

```js file="./ErrorStateFooterExample.tsx"

Expand Down
10 changes: 8 additions & 2 deletions ...ocs/content/extensions/component-groups/examples/NotAuthorized/NotAuthorized.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,14 @@ A **not authorized** component displays an error screen to users when they attem

A basic not authorized component displays a title, a description, and custom actions.

You can customize the not authorized component to fit your use case by specifying the `serviceName` to appear in the title, the `description` to provide appropriate context for the error, and the `actions` that a user can take instead.
```js file="./NotAuthorizedDefaultExample.tsx"

```js file="./NotAuthorizedExample.tsx"
```

### Not authorized with custom actions

You can customize the not authorized component to fit your use case by specifying the `serviceName` to appear in the title, a `description` of appropriate context for the error, and the `actions` that a user can take instead.

```js file="./NotAuthorizedCustomExample.tsx"

```
3 changes: 1 addition & 2 deletions ...es/NotAuthorized/NotAuthorizedExample.tsx → ...Authorized/NotAuthorizedCustomExample.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import NotAuthorized from "@patternfly/react-component-groups/dist/dynamic/NotAu
export const BasicExample: React.FunctionComponent = () => {
const primaryAction =
<Button key="1">
First action
Custom primary action
</Button>;
const secondaryActions = [
<Button key="2" variant="link">
Expand All @@ -19,7 +19,6 @@ export const BasicExample: React.FunctionComponent = () => {
<NotAuthorized
primaryAction={primaryAction}
secondaryActions={secondaryActions}
className="something"
description="Description text"
serviceName="Demo bundle"
prevPageButtonText="Go to previous page"
Expand Down
11 changes: 11 additions & 0 deletions ...ontent/extensions/component-groups/examples/NotAuthorized/NotAuthorizedDefaultExample.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import React from 'react';
import NotAuthorized from "@patternfly/react-component-groups/dist/dynamic/NotAuthorized";

export const BasicExample: React.FunctionComponent = () => (
<NotAuthorized
description="Description text"
serviceName="Demo bundle"
prevPageButtonText="Go to previous page"
/>
);

12 changes: 7 additions & 5 deletions ...-docs/content/extensions/component-groups/examples/WarningModal/WarningModal.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,26 @@ A **warning modal** component displays a modal when a user tries to perform a ri

A basic warning modal is triggered when a user tries to perform an action that is deemed risky.

You can customize the contents of the modal to fit your use cases. To adjust the text in the modal, pass your desired title to `title` and your message to the `<WarningModal>` component. To customize the action buttons in the modal, use `onConfirm` and `onClose`.
You can customize the contents of the modal to fit your use cases. To adjust the text in the modal, pass your desired title to `title` and your message to the `<WarningModal>` component. To customize the action buttons in the modal, use `onConfirm` and `onClose`.

For further customization, you can utilize all properties of the [modal component](/components/modal), with the exception of `ref`.

```js file="./WarningModalExample.tsx"

```

### Warning Modal with danger button variant
### Warning modal with a custom button variant

You can customize the modal to have a confirm button with a danger color
You can apply custom variant to a warning modal's confirmation button.

```js file="./WarningModalDangerExample.tsx"

```


### Warning Modal with checkbox
### Warning modal with a checkbox

You can customize the Warning Modal to include a checkbox to double check the user wants to use the selected action.
To confirm that a user wishes to initiate a selected action, you can add a checkbox to a warning modal.

```js file="./WarningModalCheckboxExample.tsx"

Expand Down
27 changes: 9 additions & 18 deletions packages/module/src/CloseButton/CloseButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,32 @@
import React from 'react';
import { Button, ButtonProps, ButtonVariant } from '@patternfly/react-core';
import { CloseIcon } from '@patternfly/react-icons';
import clsx from 'clsx';
import { createUseStyles } from 'react-jss';
import clsx from 'clsx';

const useStyles = createUseStyles({
Copy link
Contributor

@nicolethoen nicolethoen Jan 31, 2024

Choose a reason for hiding this comment

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

@jessiehuff does removing the padding on this component keep the click area large enough to meet a11y standards?

And I think that in general, to keep these components compatible with right-to-left languages, we might want to avoid using float:right. I think there is a float:end or something like that we should be using. I'm also curious though if there's a PF modifier we could use rather than this custom styling. @mcoker WDYT?

Copy link

@mcoker mcoker Jan 31, 2024

Choose a reason for hiding this comment

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

@nicolethoen good call on the RTL support. There are logical values for float - "inline-end" (used instead of "right" - what you'd use here) and "inline-start" (used instead of "left").

I'm also curious though if there's a PF modifier we could use rather than this custom styling

We don't have a global modifier for this. We do have float utility classes though, and that utility stylesheet is lightweight (https://unpkg.com/browse/@patternfly/patternfly@latest/utilities/Float/float.css) if that's an option, though it's worth noting that our utility classes are not using logical values when they probably should. I opened an issue for that here patternfly/patternfly#6273.

Copy link
Contributor

@jessiehuff jessiehuff Feb 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Based on WCAG guidelines, the clickable area should be 44 x 44 pixels. It looks like removing the padding makes it 48 x 36 so we should probably at least increase the height to 44 pixels.

Copy link
Collaborator

@dlabaj dlabaj Feb 5, 2024

Choose a reason for hiding this comment

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

i think we are going to need a designer to step in at some point to commit a review once or twice a months of new components. with the changes in v6 I can see this giving us headaches. Fortunately we are small enough right now w can fix stuff.

'& float-right': {
float: 'right'
closeButton: {
float: 'inline-end',
padding: '10px 14px'
},
'& no-padding': {
padding: 0
}
});

export interface CloseButtonProps extends ButtonProps {
/** Additional styling to apply to the close button. */
additionalClassName?: string;
/** Aria label for accessibility */
ariaLabel?: string;
/** Data test id used for testing. */
/** Data test ID used for testing. */
dataTestID?: string;
/** Callback when the button is clicked*/
onClick: (event: any) => void;
}
};

const CloseButton: React.FunctionComponent<CloseButtonProps> = ({
additionalClassName,
ariaLabel,
className,
dataTestID,
onClick,
...props
}: CloseButtonProps) => {
const classes = useStyles();
return (
<Button
aria-label={ariaLabel || 'Close'}
className={clsx(classes, additionalClassName)}
aria-label={props['aria-label'] || 'Close'}
className={clsx(classes.closeButton, className)}
data-test-id={dataTestID}
onClick={onClick}
variant={ButtonVariant.plain}
Expand Down
12 changes: 6 additions & 6 deletions packages/module/src/NotAuthorized/NotAuthorized.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { Button, EmptyState, EmptyStateBody, EmptyStateIcon, EmptyStateProps, EmptyStateVariant, EmptyStateHeader, EmptyStateFooter, EmptyStateActions, } from '@patternfly/react-core';
import { Button, EmptyState, EmptyStateBody, EmptyStateIcon, EmptyStateProps, EmptyStateVariant, EmptyStateHeader, EmptyStateFooter, EmptyStateActions, ButtonVariant, } from '@patternfly/react-core';
import { LockIcon } from '@patternfly/react-icons';

export interface NotAuthorizedProps extends Omit<EmptyStateProps, 'children' | 'title'> {
Expand Down Expand Up @@ -46,18 +46,18 @@ const NotAuthorized: React.FunctionComponent<NotAuthorizedProps> = ({
<EmptyStateBody>{description}</EmptyStateBody>
<EmptyStateFooter>
{primaryAction ? <EmptyStateActions>{primaryAction}</EmptyStateActions> : null}
<EmptyStateActions>
{secondaryActions ? <EmptyStateActions>{secondaryActions}</EmptyStateActions> : null}
{showReturnButton &&
{showReturnButton && !primaryAction &&
(document.referrer ? (
<Button variant="link" onClick={() => history.back()}>
<Button variant={ButtonVariant.primary} onClick={() => history.back()}>
{prevPageButtonText}
</Button>
) : (
<Button variant="link" component="a" href={toLandingPageUrl}>
<Button variant={ButtonVariant.primary} component="a" href={toLandingPageUrl}>
{toLandingPageText}
</Button>
))}
<EmptyStateActions>
{secondaryActions ? <EmptyStateActions>{secondaryActions}</EmptyStateActions> : null}
</EmptyStateActions>
</EmptyStateFooter>
</EmptyState>
Expand Down
125 changes: 55 additions & 70 deletions packages/module/src/NotAuthorized/__snapshots__/NotAuthorized.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,20 +45,19 @@ exports[`NotAuthorized component should apply custom styles 1`] = `
<div
class="pf-v5-c-empty-state__footer"
>
<a
aria-disabled="false"
class="pf-v5-c-button pf-m-primary"
data-ouia-component-id="OUIA-Generated-Button-primary-2"
data-ouia-component-type="PF5/Button"
data-ouia-safe="true"
href="."
>
Go to landing page
</a>
<div
class="pf-v5-c-empty-state__actions"
>
<a
aria-disabled="false"
class="pf-v5-c-button pf-m-link"
data-ouia-component-id="OUIA-Generated-Button-link-2"
data-ouia-component-type="PF5/Button"
data-ouia-safe="true"
href="."
>
Go to landing page
</a>
</div>
/>
</div>
</div>
</div>
Expand Down Expand Up @@ -162,20 +161,19 @@ exports[`NotAuthorized component should render 1`] = `
<div
class="pf-v5-c-empty-state__footer"
>
<a
aria-disabled="false"
class="pf-v5-c-button pf-m-primary"
data-ouia-component-id="OUIA-Generated-Button-primary-1"
data-ouia-component-type="PF5/Button"
data-ouia-safe="true"
href="."
>
Go to landing page
</a>
<div
class="pf-v5-c-empty-state__actions"
>
<a
aria-disabled="false"
class="pf-v5-c-button pf-m-link"
data-ouia-component-id="OUIA-Generated-Button-link-1"
data-ouia-component-type="PF5/Button"
data-ouia-safe="true"
href="."
>
Go to landing page
</a>
</div>
/>
</div>
</div>
</div>
Expand Down Expand Up @@ -252,16 +250,6 @@ exports[`NotAuthorized component should show custom actions 1`] = `
3
</button>
</div>
<a
aria-disabled="false"
class="pf-v5-c-button pf-m-link"
data-ouia-component-id="OUIA-Generated-Button-link-6"
data-ouia-component-type="PF5/Button"
data-ouia-safe="true"
href="."
>
Go to landing page
</a>
</div>
</div>
</div>
Expand Down Expand Up @@ -313,20 +301,19 @@ exports[`NotAuthorized component should show custom description 1`] = `
<div
class="pf-v5-c-empty-state__footer"
>
<a
aria-disabled="false"
class="pf-v5-c-button pf-m-primary"
data-ouia-component-id="OUIA-Generated-Button-primary-4"
data-ouia-component-type="PF5/Button"
data-ouia-safe="true"
href="."
>
Go to landing page
</a>
<div
class="pf-v5-c-empty-state__actions"
>
<a
aria-disabled="false"
class="pf-v5-c-button pf-m-link"
data-ouia-component-id="OUIA-Generated-Button-link-4"
data-ouia-component-type="PF5/Button"
data-ouia-safe="true"
href="."
>
Go to landing page
</a>
</div>
/>
</div>
</div>
</div>
Expand Down Expand Up @@ -377,20 +364,19 @@ exports[`NotAuthorized component should show custom title 1`] = `
<div
class="pf-v5-c-empty-state__footer"
>
<a
aria-disabled="false"
class="pf-v5-c-button pf-m-primary"
data-ouia-component-id="OUIA-Generated-Button-primary-5"
data-ouia-component-type="PF5/Button"
data-ouia-safe="true"
href="."
>
Go to landing page
</a>
<div
class="pf-v5-c-empty-state__actions"
>
<a
aria-disabled="false"
class="pf-v5-c-button pf-m-link"
data-ouia-component-id="OUIA-Generated-Button-link-5"
data-ouia-component-type="PF5/Button"
data-ouia-safe="true"
href="."
>
Go to landing page
</a>
</div>
/>
</div>
</div>
</div>
Expand Down Expand Up @@ -441,20 +427,19 @@ exports[`NotAuthorized component should use custom icon 1`] = `
<div
class="pf-v5-c-empty-state__footer"
>
<a
aria-disabled="false"
class="pf-v5-c-button pf-m-primary"
data-ouia-component-id="OUIA-Generated-Button-primary-3"
data-ouia-component-type="PF5/Button"
data-ouia-safe="true"
href="."
>
Go to landing page
</a>
<div
class="pf-v5-c-empty-state__actions"
>
<a
aria-disabled="false"
class="pf-v5-c-button pf-m-link"
data-ouia-component-id="OUIA-Generated-Button-link-3"
data-ouia-component-type="PF5/Button"
data-ouia-safe="true"
href="."
>
Go to landing page
</a>
</div>
/>
</div>
</div>
</div>
Expand Down
Loading