From 801ca9653154490f664e55bbb357cd6cf6f198be Mon Sep 17 00:00:00 2001 From: Josh Black Date: Thu, 25 Jul 2024 09:59:32 -0500 Subject: [PATCH] feat(react): add ScrollableRegion and useOverflow (#4719) * feat(react): add ScrollableRegion and useOverflow * chore: address ts error in Table * test: update snapshots * chore: add changeset --------- Co-authored-by: Josh Black --- .changeset/thick-ants-try.md | 5 ++ package-lock.json | 8 +- packages/react/src/DataTable/Table.tsx | 4 +- 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 + .../__snapshots__/exports.test.ts.snap | 6 ++ packages/react/src/drafts/hooks/index.ts | 1 + packages/react/src/drafts/index.ts | 3 + .../src/{internal => }/hooks/useOverflow.ts | 10 ++- 13 files changed, 202 insertions(+), 17 deletions(-) create mode 100644 .changeset/thick-ants-try.md 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/.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 diff --git a/package-lock.json b/package-lock.json index cb1a959ea15..7dc8d28bff1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -288,7 +288,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", @@ -307,7 +307,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", @@ -350,7 +350,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", @@ -58974,7 +58974,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/DataTable/Table.tsx b/packages/react/src/DataTable/Table.tsx index f1b9edf9dc7..e6bd49a6d21 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 @@ -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 + +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/__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", 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 dcc43646db4..16a78befb4a 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 + } } })