From 1d133d50ab5ae93fee5db1a2668a3d5950b8dc86 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 3 Jul 2024 13:02:25 -0500 Subject: [PATCH 1/4] feat(react): add ScrollableRegion and useOverflow --- packages/react/src/DataTable/Table.tsx | 2 +- packages/react/src/Dialog/Dialog.tsx | 2 +- packages/react/src/PageLayout/PageLayout.tsx | 2 +- .../ScrollableRegion.stories.tsx | 58 ++++++++++++ .../ScrollableRegion.test.tsx | 88 +++++++++++++++++++ .../ScrollableRegion.tsx | 30 +++++-- packages/react/src/ScrollableRegion/index.ts | 2 + packages/react/src/drafts/hooks/index.ts | 1 + packages/react/src/drafts/index.ts | 3 + .../src/{internal => }/hooks/useOverflow.ts | 10 ++- 10 files changed, 185 insertions(+), 13 deletions(-) create mode 100644 packages/react/src/ScrollableRegion/ScrollableRegion.stories.tsx create mode 100644 packages/react/src/ScrollableRegion/ScrollableRegion.test.tsx rename packages/react/src/{internal/components => ScrollableRegion}/ScrollableRegion.tsx (55%) create mode 100644 packages/react/src/ScrollableRegion/index.ts rename packages/react/src/{internal => }/hooks/useOverflow.ts (78%) diff --git a/packages/react/src/DataTable/Table.tsx b/packages/react/src/DataTable/Table.tsx index f1b9edf9dc7..a96457aec54 100644 --- a/packages/react/src/DataTable/Table.tsx +++ b/packages/react/src/DataTable/Table.tsx @@ -13,7 +13,7 @@ import type {UniqueRow} from './row' import {SortDirection} from './sorting' import {useTableLayout} from './useTable' import {SkeletonText} from '../drafts/Skeleton/SkeletonText' -import {ScrollableRegion} from '../internal/components/ScrollableRegion' +import {ScrollableRegion} from '../ScrollableRegion' // ---------------------------------------------------------------------------- // Table diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index 40026326b58..6ff1ab2c60b 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -15,7 +15,7 @@ import {FocusKeys} from '@primer/behaviors' import Portal from '../Portal' import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef' import {useId} from '../hooks/useId' -import {ScrollableRegion} from '../internal/components/ScrollableRegion' +import {ScrollableRegion} from '../ScrollableRegion' import type {ResponsiveValue} from '../hooks/useResponsiveValue' /* Dialog Version 2 */ diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index e5123fa4971..994f3d007a6 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -10,7 +10,7 @@ import type {BetterSystemStyleObject, SxProp} from '../sx' import {merge} from '../sx' import type {Theme} from '../ThemeProvider' import {canUseDOM} from '../utils/environment' -import {useOverflow} from '../internal/hooks/useOverflow' +import {useOverflow} from '../hooks/useOverflow' import {warning} from '../utils/warning' import {useStickyPaneHeight} from './useStickyPaneHeight' diff --git a/packages/react/src/ScrollableRegion/ScrollableRegion.stories.tsx b/packages/react/src/ScrollableRegion/ScrollableRegion.stories.tsx new file mode 100644 index 00000000000..c4579b493b3 --- /dev/null +++ b/packages/react/src/ScrollableRegion/ScrollableRegion.stories.tsx @@ -0,0 +1,58 @@ +import React from 'react' +import type {Meta, StoryObj} from '@storybook/react' +import {ScrollableRegion} from '../ScrollableRegion' + +const meta = { + title: 'Drafts/Components/ScrollableRegion', + component: ScrollableRegion, +} satisfies Meta + +export default meta + +export const Default = () => { + return ( + +

Example content that triggers overflow.

+

+ The content here will not wrap at smaller screen sizes and will trigger the component to set the container as a + region, label it, make it focusable, and make it scrollable. +

+ + ) +} + +export const Playground: StoryObj = { + render: args => { + return ( + +

Example content that triggers overflow.

+

+ The content here will not wrap at smaller screen sizes and will trigger the component to set the container as + a region, label it, make it focusable, and make it scrollable. +

+ + ) + }, + args: { + 'aria-label': 'Example scrollable region', + }, + argTypes: { + 'aria-label': { + control: 'text', + }, + 'aria-labelledby': { + control: 'text', + }, + className: { + control: 'text', + }, + }, +} diff --git a/packages/react/src/ScrollableRegion/ScrollableRegion.test.tsx b/packages/react/src/ScrollableRegion/ScrollableRegion.test.tsx new file mode 100644 index 00000000000..f1dc81132a0 --- /dev/null +++ b/packages/react/src/ScrollableRegion/ScrollableRegion.test.tsx @@ -0,0 +1,88 @@ +import {render, screen} from '@testing-library/react' +import React, {act} from 'react' +import {ScrollableRegion} from '../ScrollableRegion' + +const originalResizeObserver = global.ResizeObserver + +describe('ScrollableRegion', () => { + let mockResizeCallback: (entries: Array) => void + + beforeEach(() => { + global.ResizeObserver = class ResizeObserver { + constructor(callback: ResizeObserverCallback) { + mockResizeCallback = (entries: Array) => { + return callback(entries, this) + } + } + + observe() {} + disconnect() {} + unobserve() {} + } + }) + + afterEach(() => { + global.ResizeObserver = originalResizeObserver + }) + + test('does not render with region props by default', () => { + render( + + Example content + , + ) + + expect(screen.getByTestId('container')).not.toHaveAttribute('role') + expect(screen.getByTestId('container')).not.toHaveAttribute('tabindex') + expect(screen.getByTestId('container')).not.toHaveAttribute('aria-labelledby') + expect(screen.getByTestId('container')).not.toHaveAttribute('aria-label') + + expect(screen.getByTestId('container')).toHaveStyleRule('overflow', 'auto') + expect(screen.getByTestId('container')).toHaveStyleRule('position', 'relative') + }) + + test('does render with region props when overflow is present', () => { + render( + + Example content + , + ) + + act(() => { + // Mock a resize occurring when the scroll height is greater than the + // client height + const target = document.createElement('div') + mockResizeCallback([ + { + target: { + ...target, + scrollHeight: 500, + clientHeight: 100, + }, + borderBoxSize: [], + contentBoxSize: [], + contentRect: { + width: 0, + height: 0, + top: 0, + right: 0, + bottom: 0, + left: 0, + x: 0, + y: 0, + toJSON() { + return {} + }, + }, + devicePixelContentBoxSize: [], + }, + ]) + }) + + expect(screen.getByLabelText('Example label')).toBeVisible() + + expect(screen.getByLabelText('Example label')).toHaveAttribute('role', 'region') + expect(screen.getByLabelText('Example label')).toHaveAttribute('tabindex', '0') + expect(screen.getByLabelText('Example label')).toHaveAttribute('aria-label') + }) +}) diff --git a/packages/react/src/internal/components/ScrollableRegion.tsx b/packages/react/src/ScrollableRegion/ScrollableRegion.tsx similarity index 55% rename from packages/react/src/internal/components/ScrollableRegion.tsx rename to packages/react/src/ScrollableRegion/ScrollableRegion.tsx index 90e03ecf58f..e1c2ca12d52 100644 --- a/packages/react/src/internal/components/ScrollableRegion.tsx +++ b/packages/react/src/ScrollableRegion/ScrollableRegion.tsx @@ -1,26 +1,39 @@ import React from 'react' -import Box from '../../Box' +import Box from '../Box' import {useOverflow} from '../hooks/useOverflow' -type ScrollableRegionProps = React.PropsWithChildren<{ - 'aria-labelledby'?: string - className?: string -}> +type Labelled = + | { + 'aria-label': string + 'aria-labelledby'?: never + } + | { + 'aria-label'?: never + 'aria-labelledby': string + } + +type ScrollableRegionProps = React.ComponentPropsWithoutRef<'div'> & Labelled const defaultStyles = { // When setting overflow, we also set `position: relative` to avoid // `position: absolute` items breaking out of the container and causing - // scrollabrs on the page. This can occur with common classes like `sr-only` + // scrollbars on the page. This can occur with common classes like `sr-only` // and can cause difficult to track down layout issues position: 'relative', overflow: 'auto', } -export function ScrollableRegion({'aria-labelledby': labelledby, children, ...rest}: ScrollableRegionProps) { +function ScrollableRegion({ + 'aria-label': label, + 'aria-labelledby': labelledby, + children, + ...rest +}: ScrollableRegionProps) { const ref = React.useRef(null) const hasOverflow = useOverflow(ref) const regionProps = hasOverflow ? { + 'aria-label': label, 'aria-labelledby': labelledby, role: 'region', tabIndex: 0, @@ -33,3 +46,6 @@ export function ScrollableRegion({'aria-labelledby': labelledby, children, ...re ) } + +export {ScrollableRegion} +export type {ScrollableRegionProps} diff --git a/packages/react/src/ScrollableRegion/index.ts b/packages/react/src/ScrollableRegion/index.ts new file mode 100644 index 00000000000..bdb8d051a87 --- /dev/null +++ b/packages/react/src/ScrollableRegion/index.ts @@ -0,0 +1,2 @@ +export {ScrollableRegion} from './ScrollableRegion' +export type {ScrollableRegionProps} from './ScrollableRegion' diff --git a/packages/react/src/drafts/hooks/index.ts b/packages/react/src/drafts/hooks/index.ts index 4bf219f85e8..e5eab8c739f 100644 --- a/packages/react/src/drafts/hooks/index.ts +++ b/packages/react/src/drafts/hooks/index.ts @@ -4,3 +4,4 @@ export * from './useIgnoreKeyboardActionsWhileComposing' export * from './useSafeAsyncCallback' export * from './useSyntheticChange' export * from '../../hooks/useSlots' +export {useOverflow} from '../../hooks/useOverflow' diff --git a/packages/react/src/drafts/index.ts b/packages/react/src/drafts/index.ts index fca323ac256..feb996f7c29 100644 --- a/packages/react/src/drafts/index.ts +++ b/packages/react/src/drafts/index.ts @@ -72,6 +72,9 @@ export type {TabPanelsProps, TabPanelsTabProps, TabPanelsPanelProps} from './Tab export * from '../TooltipV2' export * from '../ActionBar' +export {ScrollableRegion} from '../ScrollableRegion' +export type {ScrollableRegionProps} from '../ScrollableRegion' + export {Stack} from '../Stack' export type {StackProps, StackItemProps} from '../Stack' diff --git a/packages/react/src/internal/hooks/useOverflow.ts b/packages/react/src/hooks/useOverflow.ts similarity index 78% rename from packages/react/src/internal/hooks/useOverflow.ts rename to packages/react/src/hooks/useOverflow.ts index 9b027c021ce..0c394d095e5 100644 --- a/packages/react/src/internal/hooks/useOverflow.ts +++ b/packages/react/src/hooks/useOverflow.ts @@ -10,9 +10,13 @@ export function useOverflow(ref: React.RefObject) { const observer = new ResizeObserver(entries => { for (const entry of entries) { - setHasOverflow( - entry.target.scrollHeight > entry.target.clientHeight || entry.target.scrollWidth > entry.target.clientWidth, - ) + if ( + entry.target.scrollHeight > entry.target.clientHeight || + entry.target.scrollWidth > entry.target.clientWidth + ) { + setHasOverflow(true) + break + } } }) From 3eba998af1209af3ccce67c33b1b557f1bd9c30a Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 3 Jul 2024 15:47:29 -0500 Subject: [PATCH 2/4] chore: address ts error in Table --- packages/react/src/DataTable/Table.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react/src/DataTable/Table.tsx b/packages/react/src/DataTable/Table.tsx index a96457aec54..e6bd49a6d21 100644 --- a/packages/react/src/DataTable/Table.tsx +++ b/packages/react/src/DataTable/Table.tsx @@ -250,6 +250,8 @@ const Table = React.forwardRef(function Table( ref, ) { return ( + // TODO update type to be non-optional in next major release + // @ts-expect-error this type should be required in the next major version Date: Tue, 23 Jul 2024 10:29:53 -0500 Subject: [PATCH 3/4] test: update snapshots --- package-lock.json | 8 ++++---- .../src/__tests__/__snapshots__/exports.test.ts.snap | 6 ++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index e942c8990d0..648f84682f3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -287,7 +287,7 @@ "examples/app-router": { "name": "example-app-router", "dependencies": { - "@primer/react": "36.25.0", + "@primer/react": "36.26.0", "next": "^14.1.0", "react": "^18.3.1", "react-dom": "^18.3.1", @@ -306,7 +306,7 @@ "react-dom": "^18.3.1" }, "devDependencies": { - "@primer/react": "36.25.0", + "@primer/react": "36.26.0", "@types/react": "^18.3.3", "@types/react-dom": "^18.3.0", "@typescript-eslint/eslint-plugin": "^7.11.0", @@ -349,7 +349,7 @@ "version": "0.0.0", "dependencies": { "@primer/octicons-react": "19.x", - "@primer/react": "36.25.0", + "@primer/react": "36.26.0", "next": "^14.1.0", "react": "^18.3.1", "react-dom": "^18.3.1", @@ -58973,7 +58973,7 @@ }, "packages/react": { "name": "@primer/react", - "version": "36.25.0", + "version": "36.26.0", "license": "MIT", "dependencies": { "@github/combobox-nav": "^2.1.5", diff --git a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap index a48662d7968..0273c0fe29e 100644 --- a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap +++ b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap @@ -301,6 +301,8 @@ exports[`@primer/react/drafts should not update exports without a semver change "type ParentLinkProps", "type Reference", "type SavedReply", + "ScrollableRegion", + "type ScrollableRegionProps", "SelectPanel", "type SelectPanelMessageProps", "type SelectPanelProps", @@ -345,6 +347,7 @@ exports[`@primer/react/drafts should not update exports without a semver change "useCombobox", "useDynamicTextareaHeight", "useIgnoreKeyboardActionsWhileComposing", + "useOverflow", "useSafeAsyncCallback", "useSlots", "useSyntheticChange", @@ -414,6 +417,8 @@ exports[`@primer/react/experimental should not update exports without a semver c "type ParentLinkProps", "type Reference", "type SavedReply", + "ScrollableRegion", + "type ScrollableRegionProps", "SelectPanel", "type SelectPanelMessageProps", "type SelectPanelProps", @@ -458,6 +463,7 @@ exports[`@primer/react/experimental should not update exports without a semver c "useCombobox", "useDynamicTextareaHeight", "useIgnoreKeyboardActionsWhileComposing", + "useOverflow", "useSafeAsyncCallback", "useSlots", "useSyntheticChange", From d1270a62d850e4b3a315b93e13a7b291a705c35d Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 23 Jul 2024 10:30:21 -0500 Subject: [PATCH 4/4] chore: add changeset --- .changeset/thick-ants-try.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/thick-ants-try.md diff --git a/.changeset/thick-ants-try.md b/.changeset/thick-ants-try.md new file mode 100644 index 00000000000..66d4bd2ee81 --- /dev/null +++ b/.changeset/thick-ants-try.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Add experimental ScrollableRegion component and useOverflow hook