Skip to content

Commit

Permalink
Merge branch 'main' into form-control-required-indicator
Browse files Browse the repository at this point in the history
  • Loading branch information
TylerJDev authored May 9, 2024
2 parents 5aa4144 + 024124a commit 0d0b531
Show file tree
Hide file tree
Showing 28 changed files with 218 additions and 21 deletions.
5 changes: 5 additions & 0 deletions .changeset/mean-terms-bathe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

ActionBar: Add a few fixes and relevant tests
5 changes: 5 additions & 0 deletions .changeset/popular-jokes-kiss.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

`Dialog` and `ConfirmationDialog` can now be closed by clicking on the backdrop surrounding the dialog. This will cause `onClose` to be called with a new `'backdrop'` gesture.
5 changes: 5 additions & 0 deletions .changeset/quiet-lamps-kiss.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

TreeView: Fix toggling subtree via Space key (in addition to Enter key)
5 changes: 5 additions & 0 deletions .changeset/tiny-ghosts-notice.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

Exports createComponent
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.
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.
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.
14 changes: 7 additions & 7 deletions docs/content/drafts/Dialog.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,13 @@ render(<ShorthandExample2 />)
### ConfirmationDialogProps
| Prop name | Type | Default | Description |
| :------------------- | :-------------------------------------------------------------------- | :--------- | :---------------------------------------------------------------------------------------------------------------------------- |
| title | `React.ReactNode` | | Required. Sets the title of the dialog, which by default is also used as the `aria-labelledby` attribute. |
| onClose | `(gesture: 'confirm''cancel''close-button''escape') => void` | | Required. This callback is invoked when a gesture to close the dialog is performed. The first argument indicates the gesture. |
| cancelButtonContent | `React.ReactNode` | `"Cancel"` | The content to use for the cancel button. |
| confirmButtonContent | `React.ReactNode` | `"OK"` | The content to use for the confirm button. |
| confirmButtonType | `"normal""primary""danger"` | `Button` | The type of button to use for the confirm button. |
| Prop name | Type | Default | Description |
| :------------------- | :----------------------------------------------- | :----------------------------- | :-------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- |
| title | `React.ReactNode` | | Required. Sets the title of the dialog, which by default is also used as the `aria-labelledby` attribute. |
| onClose | `(gesture: 'confirm''cancel''close-button' | 'backdrop 'escape') => void` | | Required. This callback is invoked when a gesture to close the dialog is performed. The first argument indicates the gesture. |
| cancelButtonContent | `React.ReactNode` | `"Cancel"` | The content to use for the cancel button. |
| confirmButtonContent | `React.ReactNode` | `"OK"` | The content to use for the confirm button. |
| confirmButtonType | `"normal" │ "primary" │ "danger"` | `Button` | The type of button to use for the confirm button. |
### ConfirmOptions
Expand Down
54 changes: 54 additions & 0 deletions e2e/components/drafts/ActionBar.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import {test, expect} from '@playwright/test'
import {visit} from '../../test-helpers/storybook'
import {themes} from '../../test-helpers/themes'
import {viewports} from '../../test-helpers/viewports'

test.describe('ActionBar', () => {
test.describe('Default state', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'drafts-components-actionbar--comment-box',
globals: {
colorScheme: theme,
},
})
expect(await page.screenshot()).toMatchSnapshot(`drafts.ActionBar.CommentBox.${theme}.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'drafts-components-actionbar--comment-box',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations()
})
})
}
})

test.describe('ActionBar Interactions', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('Overflow interaction @vrt', async ({page}) => {
await visit(page, {
id: 'drafts-components-actionbar--comment-box',
globals: {
colorScheme: theme,
},
})
const toolbarButtonSelector = `button[data-component="IconButton"]`
await expect(page.locator(toolbarButtonSelector)).toHaveCount(10)
await page.setViewportSize({width: viewports['primer.breakpoint.xs'], height: 768})
await expect(page.locator(toolbarButtonSelector)).toHaveCount(6)
const moreButtonSelector = `button[aria-label="More Comment box toolbar items"]`
await page.locator(moreButtonSelector).click()
await expect(page.locator('ul[role="menu"]>li')).toHaveCount(5)
})
})
}
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface ConfirmationDialogProps {
* Required. This callback is invoked when a gesture to close the dialog
* is performed. The first argument indicates the gesture.
*/
onClose: (gesture: 'confirm' | 'close-button' | 'cancel' | 'escape') => void
onClose: (gesture: 'confirm' | 'close-button' | 'backdrop' | 'cancel' | 'escape') => void

/**
* Required. The title of the ConfirmationDialog. This is usually a brief
Expand Down
19 changes: 17 additions & 2 deletions packages/react/src/Dialog/Dialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,22 @@ describe('Dialog', () => {

await user.click(getByLabelText('Close'))

expect(onClose).toHaveBeenCalled()
expect(onClose).toHaveBeenCalledWith('close-button')
expect(onClose).toHaveBeenCalledTimes(1) // Ensure it's not called with a backdrop gesture as well
})

it('calls `onClose` when clicking the backdrop', async () => {
const user = userEvent.setup()
const onClose = jest.fn()
const {getByRole} = render(<Dialog onClose={onClose}>Pay attention to me</Dialog>)

expect(onClose).not.toHaveBeenCalled()

const dialog = getByRole('dialog')
const backdrop = dialog.parentElement!
await user.click(backdrop)

expect(onClose).toHaveBeenCalledWith('backdrop')
})

it('calls `onClose` when keying "Escape"', async () => {
Expand All @@ -80,7 +95,7 @@ describe('Dialog', () => {

await user.keyboard('{Escape}')

expect(onClose).toHaveBeenCalled()
expect(onClose).toHaveBeenCalledWith('escape')
})

it('changes the <body> style for `overflow` if it is not set to "hidden"', () => {
Expand Down
18 changes: 13 additions & 5 deletions packages/react/src/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useCallback, useEffect, useRef, useState} from 'react'
import React, {useCallback, useEffect, useRef, useState, type SyntheticEvent} from 'react'
import styled from 'styled-components'
import type {ButtonProps} from '../Button'
import {Button} from '../Button'
Expand Down Expand Up @@ -98,11 +98,11 @@ export interface DialogProps extends SxProp {

/**
* This method is invoked when a gesture to close the dialog is used (either
* an Escape key press or clicking the "X" in the top-right corner). The
* an Escape key press, clicking the backdrop, or clicking the "X" in the top-right corner). The
* gesture argument indicates the gesture that was used to close the dialog
* (either 'close-button' or 'escape').
* ('close-button', 'backdrop', or 'escape').
*/
onClose: (gesture: 'close-button' | 'escape') => void
onClose: (gesture: 'close-button' | 'backdrop' | 'escape') => void

/**
* Default: "dialog". The ARIA role to assign to this dialog.
Expand Down Expand Up @@ -414,6 +414,14 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
}
}
const defaultedProps = {...props, title, subtitle, role, dialogLabelId, dialogDescriptionId}
const onBackdropClick = useCallback(
(e: SyntheticEvent) => {
if (e.target === e.currentTarget) {
onClose('backdrop')
}
},
[onClose],
)

const dialogRef = useRef<HTMLDivElement>(null)
useRefObjectAsForwardedRef(forwardedRef, dialogRef)
Expand Down Expand Up @@ -465,7 +473,7 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
return (
<>
<Portal>
<Backdrop ref={backdropRef} {...positionDataAttributes}>
<Backdrop ref={backdropRef} {...positionDataAttributes} onClick={onBackdropClick}>
<StyledDialog
width={width}
height={height}
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/RelativeTime/RelativeTime.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react'
import {RelativeTimeElement} from '@github/relative-time-element'
import type {ComponentProps} from '../utils/types'
import {createComponent} from '../utils/custom-element'
import {createComponent} from '../utils/create-component'

const RelativeTimeComponent = createComponent(RelativeTimeElement, 'relative-time')

Expand Down
96 changes: 96 additions & 0 deletions packages/react/src/TreeView/TreeView.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,102 @@ describe('Keyboard interactions', () => {
})
})

describe('Space', () => {
it('calls onSelect function if provided and checks if the item has been selected', () => {
const onSelect = jest.fn()
const {getByRole} = renderWithTheme(
<TreeView aria-label="Test tree">
<TreeView.Item id="parent-1" onSelect={onSelect}>
Parent 1
<TreeView.SubTree>
<TreeView.Item id="child-1" onSelect={onSelect}>
Child 1
</TreeView.Item>
</TreeView.SubTree>
</TreeView.Item>
<TreeView.Item id="parent-2" onSelect={onSelect} expanded>
Parent 2
<TreeView.SubTree>
<TreeView.Item id="child-2" onSelect={onSelect}>
Child2
</TreeView.Item>
</TreeView.SubTree>
</TreeView.Item>
<TreeView.Item id="parent-3" onSelect={onSelect}>
Parent 3
<TreeView.SubTree>
<TreeView.Item id="child-3" onSelect={onSelect}>
Child 3
</TreeView.Item>
</TreeView.SubTree>
</TreeView.Item>
</TreeView>,
)
const itemChild = getByRole('treeitem', {name: 'Child2'})

act(() => {
// Focus first item
itemChild.focus()
})

// Press Enter
fireEvent.keyDown(document.activeElement || document.body, {key: ' '})

// onSelect should have been called
expect(onSelect).toHaveBeenCalledTimes(1)

onSelect.mockClear()

// Press middle click
fireEvent.click(document.activeElement?.firstChild || document.body, {button: 1})

// onSelect should have been called
expect(onSelect).toHaveBeenCalledTimes(1)
})

it('toggles expanded state if no onSelect function is provided', () => {
const {getByRole, queryByRole} = renderWithTheme(
<TreeView aria-label="Test tree">
<TreeView.Item id="parent">
Parent
<TreeView.SubTree>
<TreeView.Item id="child-1">Child 1</TreeView.Item>
<TreeView.Item id="child-2">Child 2</TreeView.Item>
</TreeView.SubTree>
</TreeView.Item>
</TreeView>,
)

const parent = getByRole('treeitem', {name: 'Parent'})

act(() => {
// Focus first item
parent.focus()
})

// aria-expanded should be false
expect(parent).toHaveAttribute('aria-expanded', 'false')

// Press Enter
fireEvent.keyDown(document.activeElement || document.body, {key: 'Enter'})

// aria-expanded should now be true
expect(parent).toHaveAttribute('aria-expanded', 'true')

// Subtree should be visible
expect(queryByRole('group')).toBeVisible()

// Press Enter
fireEvent.keyDown(document.activeElement || document.body, {key: 'Enter'})

// aria-expanded should now be false
expect(parent).toHaveAttribute('aria-expanded', 'false')

// Subtree should no longer be visible
expect(queryByRole('group')).not.toBeInTheDocument()
})
})

describe('Typeahead', () => {
it('moves focus to the next item that matches the typed character', () => {
const {getByRole} = renderWithTheme(
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/TreeView/TreeView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ const Item = React.forwardRef<HTMLElement, TreeViewItemProps>(
(event: React.KeyboardEvent<HTMLElement>) => {
switch (event.key) {
case 'Enter':
case 'Space':
case ' ':
if (onSelect) {
onSelect(event)
} else {
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/TreeView/useTypeahead.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export function useTypeahead({containerRef, onFocusChange}: TypeaheadOptions) {

function onKeyDown(event: KeyboardEvent) {
// Ignore key presses that don't produce a character value
if (!event.key || event.key.length > 1) return
if (!event.key || event.key.length > 1 || event.key === ' ') return

// Ignore key presses that occur with a modifier
if (event.ctrlKey || event.altKey || event.metaKey) return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ exports[`@primer/react should not update exports without a semver change 1`] = `
"type ConfirmationDialogProps",
"CounterLabel",
"type CounterLabelProps",
"createComponent",
"Details",
"type DetailsProps",
"Dialog",
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/drafts/ActionBar/ActionBar.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export const CommentBox = (props: CommentBoxProps) => {
const [value, setValue] = React.useState('')
const [isOpen, setIsOpen] = React.useState(false)
const buttonRef = React.useRef(null)
const toolBarLabel = `${ariaLabel} toolbar`
const toolBarLabel = `${ariaLabel ? ariaLabel : 'Comment box'} toolbar`
return (
<Box
sx={{
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/drafts/ActionBar/ActionBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ export const ActionBarIconButton = forwardRef((props: ActionBarIconButtonProps,
const domRect = (ref as MutableRefObject<HTMLElement>).current.getBoundingClientRect()
setChildrenWidth({text, width: domRect.width})
}, [ref, setChildrenWidth])
return <IconButton ref={ref} size={size} {...props} variant="invisible" />
return <IconButton ref={ref} size={size} {...props} variant="invisible" unsafeDisableTooltip={false} />
})

const sizeToHeight = {
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/drafts/TabPanels/TabPanels.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import React from 'react'
import styled from 'styled-components'
import {get} from '../../constants'
import {TabContainerElement} from '@github/tab-container-element'
import {createComponent} from '../../utils/custom-element'
import {createComponent} from '../../utils/create-component'
import sx, {type SxProp} from '../../sx'
import type {ComponentProps} from '../../utils/types'
import getGlobalFocusStyles from '../../internal/utils/getGlobalFocusStyles'
Expand Down
3 changes: 3 additions & 0 deletions packages/react/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ export {useRefObjectAsForwardedRef} from './hooks/useRefObjectAsForwardedRef'
export {useResizeObserver} from './hooks/useResizeObserver'
export {useResponsiveValue} from './hooks/useResponsiveValue'

// Utils
export {createComponent} from './utils/create-component'

// Components
export {default as Radio} from './Radio'
export type {RadioProps} from './Radio'
Expand Down
File renamed without changes.

0 comments on commit 0d0b531

Please sign in to comment.