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: add modal button aria label and export ModalCloseButton #550

Merged
merged 3 commits into from
Jun 4, 2024
Merged

feat: add modal button aria label and export ModalCloseButton #550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bab05c3
fix: modal button aria label
yue4u Jun 3, 2024
8797c32
feat: rename and export ModalCloseButton
yue4u Jun 3, 2024
02037a7
fix: update storyshot
yue4u Jun 3, 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
3 changes: 3 additions & 0 deletions packages/react/src/components/IconButton/__snapshots__/index.story.storyshot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ exports[`Storybook Tests react/IconButton Default 1`] = `
title="add"
>
<pixiv-icon
aria-hidden="true"
name="16/Add"
/>
</button>
Expand Down Expand Up @@ -211,6 +212,7 @@ exports[`Storybook Tests react/IconButton IsActive 1`] = `
className="c0 c1"
>
<pixiv-icon
aria-hidden="true"
name="16/Add"
/>
</button>
Expand Down Expand Up @@ -322,6 +324,7 @@ exports[`Storybook Tests react/IconButton Overlay 1`] = `
className="c0 c1"
>
<pixiv-icon
aria-hidden="true"
name="16/Add"
/>
</button>
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/components/IconButton/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const IconButton = forwardRef<ClickableElement, IconButtonProps>(
$variant={variant}
$isActive={isActive}
>
<pixiv-icon name={icon} />
<pixiv-icon aria-hidden="true" name={icon} />
Copy link
Contributor Author

@yue4u yue4u Jun 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This changes all IconButtons. If it's too broad to decide I'll make this a separate PR.

</StyledIconButton>
)
}
Expand Down
12 changes: 12 additions & 0 deletions packages/react/src/components/Modal/__snapshots__/index.story.storyshot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -940,10 +940,13 @@ exports[`Storybook Tests react/Modal BackgroundScroll 1`] = `
</div>
</div>
<button
aria-label="Close"
className="c0 c27 c28"
onClick={[Function]}
type="button"
>
<pixiv-icon
aria-hidden="true"
name="24/Close"
/>
</button>
Expand Down Expand Up @@ -1420,10 +1423,13 @@ exports[`Storybook Tests react/Modal BottomSheet 1`] = `
</div>
</div>
<button
aria-label="Close"
className="c0 c13 c14"
onClick={[Function]}
type="button"
>
<pixiv-icon
aria-hidden="true"
name="24/Close"
/>
</button>
Expand Down Expand Up @@ -2365,10 +2371,13 @@ exports[`Storybook Tests react/Modal Default 1`] = `
</div>
</div>
<button
aria-label="Close"
className="c0 c27 c28"
onClick={[Function]}
type="button"
>
<pixiv-icon
aria-hidden="true"
name="24/Close"
/>
</button>
Expand Down Expand Up @@ -3162,10 +3171,13 @@ exports[`Storybook Tests react/Modal FullBottomSheet 1`] = `
</div>
</div>
<button
aria-label="Close"
className="c0 c22 c23"
onClick={[Function]}
type="button"
>
<pixiv-icon
aria-hidden="true"
name="24/Close"
/>
</button>
Expand Down
8 changes: 6 additions & 2 deletions packages/react/src/components/Modal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export type ModalProps = AriaModalOverlayProps &
isOpen: boolean
onClose: () => void
className?: string
closeButtonAriaLabel?: string

/**
* https://github.com/adobe/react-spectrum/issues/3787
Expand Down Expand Up @@ -74,6 +75,7 @@ const Modal = forwardRef<HTMLDivElement, ModalProps>(function ModalInner(
onClose,
className,
isOpen = false,
closeButtonAriaLabel = 'Close',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

make this a prop for i18n

} = props

const ref = useObjectRef<HTMLDivElement>(external)
Expand Down Expand Up @@ -162,9 +164,11 @@ const Modal = forwardRef<HTMLDivElement, ModalProps>(function ModalInner(
>
{children}
{isDismissable === true && (
<ModalCrossButton
<ModalCloseButton
size="S"
icon="24/Close"
type="button"
aria-label={closeButtonAriaLabel}
onClick={onClose}
/>
)}
Expand Down Expand Up @@ -224,7 +228,7 @@ const ModalBackground = animated(styled.div<{
}
`)

const ModalCrossButton = styled(IconButton)`
export const ModalCloseButton = styled(IconButton)`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case user want to set isDismissable to false and handle it themselves, for example, pass different onClose for close button and background.

position: absolute;
top: 8px;
right: 8px;
Expand Down
6 changes: 5 additions & 1 deletion packages/react/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ export {
} from './components/TextField'
export { default as TextArea, type TextAreaProps } from './components/TextArea'
export { default as Icon, type IconProps } from './components/Icon'
export { default as Modal, type ModalProps } from './components/Modal'
export {
default as Modal,
type ModalProps,
ModalCloseButton,
} from './components/Modal'
export {
ModalHeader,
ModalAlign,
Expand Down
Loading