Skip to content

Commit 279136d

Browse files
committed
fix merge conflict
2 parents d97395c + 808df8a commit 279136d

81 files changed

Lines changed: 390 additions & 261 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.changeset/quiet-otters-dance.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@primer/react-brand': minor
3+
---
4+
5+
⚠️ Breaking change: Removed the `imageBackgroundColor` prop from `River.Visual`, `RiverBreakout.Visual`, and `RiverBreakoutTabs.Visual`.
6+
7+
The `gridline` variant now applies the full-bleed visual background layout by default. Remove `imageBackgroundColor="subtle"` from `River.Visual` and `RiverBreakout.Visual` when using the `gridline` variant.
8+
9+
Remove `imageBackgroundColor` from `RiverBreakoutTabs.Visual` without replacement. Its default visual treatment is unchanged.
10+
11+
The background treatment is not applied to the default `River` or `RiverBreakout` variants.
12+
13+
Updated the River `gridline` variant tablet layout with a `618px` max-width and lateral gridlines.

.changeset/river-visual-layout.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33
'@primer/brand-css': minor
44
---
55

6-
Added opt-in `position` and `padding` controls to `River.Visual` media in the `gridline` variants. Existing Rivers remain unchanged.
6+
Added opt-in `position` and `padding` controls to `River.Visual` media in the `gridline` variant.
77

88
🔗 Storybook examples:
99

10-
- [Center position](https://stunning-chainsaw-j82glqz.pages.github.io/?path=/story/components-river-features-gridline-variants--grid-line-visual-position-center)
11-
- [Block-end position](https://stunning-chainsaw-j82glqz.pages.github.io/?path=/story/components-river-features-gridline-variants--grid-line-visual-position-block-end)
12-
- [Block-end inline-start position](https://stunning-chainsaw-j82glqz.pages.github.io/?path=/story/components-river-features-gridline-variants--grid-line-visual-position-block-end-inline-start)
13-
- [Block-end inline-end position](https://stunning-chainsaw-j82glqz.pages.github.io/?path=/story/components-river-features-gridline-variants--grid-line-visual-position-block-end-inline-end)
14-
- [No padding](https://stunning-chainsaw-j82glqz.pages.github.io/?path=/story/components-river-features-gridline-variants--grid-line-visual-padding-none)
15-
- [Padding on all sides](https://stunning-chainsaw-j82glqz.pages.github.io/?path=/story/components-river-features-gridline-variants--grid-line-visual-padding-all)
10+
- [Center position](https://stunning-chainsaw-j82glqz.pages.github.io/brand/storybook/?path=/story/components-river-features-gridline-variants--grid-line-visual-position-center)
11+
- [Block-end position](https://stunning-chainsaw-j82glqz.pages.github.io/brand/storybook/?path=/story/components-river-features-gridline-variants--grid-line-visual-position-block-end)
12+
- [Block-end inline-start position](https://stunning-chainsaw-j82glqz.pages.github.io/brand/storybook/?path=/story/components-river-features-gridline-variants--grid-line-visual-position-block-end-inline-start)
13+
- [Block-end inline-end position](https://stunning-chainsaw-j82glqz.pages.github.io/brand/storybook/?path=/story/components-river-features-gridline-variants--grid-line-visual-position-block-end-inline-end)
14+
- [No padding](https://stunning-chainsaw-j82glqz.pages.github.io/brand/storybook/?path=/story/components-river-features-gridline-variants--grid-line-visual-padding-none)
15+
- [Padding on all sides](https://stunning-chainsaw-j82glqz.pages.github.io/brand/storybook/?path=/story/components-river-features-gridline-variants--grid-line-visual-padding-all)

.changeset/silver-pandas-draw.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react-brand': patch
3+
---
4+
5+
Improve `RiverAccordion` gridline layout on tablet viewports and update its accordion toggle icon from chevrons to triangles.

.changeset/tidy-traps-focus.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react-brand': minor
3+
---
4+
5+
Exported `useFocusTrap` from the package root and created stable container and initial-focus refs when they are not provided.

apps/next-docs/content/components/River/react.mdx

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,12 @@ import {River, RiverBreakout} from '@primer/react-brand'
7474

7575
### GridLine variant
7676

77-
The `gridline` variant adds lateral padding for bordered layouts. Use `imageBackgroundColor` to add contrast, `position` to align media, and `padding` to control its inset.
77+
The `gridline` variant adds lateral padding and applies a full-bleed visual background by default. Use `position` to align media and `padding` to control its inset.
7878

7979
```jsx live
8080
<Stack style={{width: '100%'}}>
8181
<River variant="gridline">
82-
<River.Visual imageBackgroundColor="subtle">
82+
<River.Visual>
8383
<img src="/images/placeholder.png" alt="placeholder, blank area with a gray background color" />
8484
</River.Visual>
8585
<River.Content>
@@ -89,7 +89,7 @@ The `gridline` variant adds lateral padding for bordered layouts. Use `imageBack
8989
</River.Content>
9090
</River>
9191
<River variant="gridline" align="end">
92-
<River.Visual imageBackgroundColor="subtle">
92+
<River.Visual>
9393
<img src="/images/placeholder.png" alt="placeholder, blank area with a gray background color" />
9494
</River.Visual>
9595
<River.Content>
@@ -99,7 +99,7 @@ The `gridline` variant adds lateral padding for bordered layouts. Use `imageBack
9999
</River.Content>
100100
</River>
101101
<River variant="gridline">
102-
<River.Visual imageBackgroundColor="subtle" position="block-end-inline-end">
102+
<River.Visual position="block-end-inline-end">
103103
<img src="/images/placeholder.png" alt="placeholder, blank area with a gray background color" />
104104
</River.Visual>
105105
<River.Content>
@@ -108,7 +108,7 @@ The `gridline` variant adds lateral padding for bordered layouts. Use `imageBack
108108
</River.Content>
109109
</River>
110110
<River variant="gridline">
111-
<River.Visual imageBackgroundColor="subtle" position="block-end-inline-start">
111+
<River.Visual position="block-end-inline-start">
112112
<img src="/images/placeholder.png" alt="placeholder, blank area with a gray background color" />
113113
</River.Visual>
114114
<River.Content>
@@ -117,7 +117,7 @@ The `gridline` variant adds lateral padding for bordered layouts. Use `imageBack
117117
</River.Content>
118118
</River>
119119
<River variant="gridline">
120-
<River.Visual imageBackgroundColor="subtle" padding="none">
120+
<River.Visual padding="none">
121121
<img src="/images/placeholder.png" alt="placeholder, blank area with a gray background color" />
122122
</River.Visual>
123123
<River.Content>
@@ -264,22 +264,21 @@ The `gridline` variant adds lateral padding for bordered layouts. Use `imageBack
264264

265265
### River.Visual <Label>Required</Label>
266266

267-
| Name | Type | Default | Description |
268-
| :--------------------- | :-------------------------- | :---------: | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
269-
| `fillMedia` | `boolean` | true | Automatically styles images and video to fill and fit the width of the parent. Disable this setting if you have bespoke styling requirements. |
270-
| `children` | `ReactElement` | | Bring your own component (BYOC) `img` or `ReactElement` (E.g. Next.js `Image` component) |
271-
| `imageBackgroundColor` | `'default'`, `'subtle'` | | Applies a background color around the media independently of visual position and padding controls. |
272-
| `position` | <RiverVisualPositionProp /> | `'default'` | Positions media within the gridline visual region. Explicit positions remain fixed across viewports; `default` preserves responsive behavior. |
273-
| `padding` | <RiverVisualPaddingProp /> | `'default'` | Controls media inset within the gridline visual region. `none` fills the visual region and removes corner radius regardless of `rounded`; `all` adds padding on every side. |
274-
| `hasShadow` | `boolean` | `false` | Shadow applied to the `children`. Set be `false` when the child node has a transparent background. |
275-
| `className` | `string` | | Sets a custom class on the root element |
276-
| `id` | `string` | | Sets a custom id |
277-
| `ref` | `React.RefObject` | | Forward a Ref to the underlying DOM node |
278-
| `rounded` | `boolean` | `true` | Toggle visually rounded corners. Enabled by default. |
267+
| Name | Type | Default | Description |
268+
| :---------- | :-------------------------- | :---------: | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
269+
| `fillMedia` | `boolean` | true | Automatically styles images and video to fill and fit the width of the parent. Disable this setting if you have bespoke styling requirements. |
270+
| `children` | `ReactElement` | | Bring your own component (BYOC) `img` or `ReactElement` (E.g. Next.js `Image` component) |
271+
| `position` | <RiverVisualPositionProp /> | `'default'` | Positions media within the gridline visual region. Explicit positions remain fixed across viewports; `default` preserves responsive behavior. |
272+
| `padding` | <RiverVisualPaddingProp /> | `'default'` | Controls media inset within the gridline visual region. `none` fills the visual region and removes corner radius regardless of `rounded`; `all` adds padding on every side. |
273+
| `hasShadow` | `boolean` | `false` | Shadow applied to the `children`. Set be `false` when the child node has a transparent background. |
274+
| `className` | `string` | | Sets a custom class on the root element |
275+
| `id` | `string` | | Sets a custom id |
276+
| `ref` | `React.RefObject` | | Forward a Ref to the underlying DOM node |
277+
| `rounded` | `boolean` | `true` | Toggle visually rounded corners. Enabled by default. |
279278

280279
### RiverBreakout.Visual <Label>Required</Label>
281280

282-
`RiverBreakout.Visual` supports the `children`, `fillMedia`, `imageBackgroundColor`, `hasShadow`, `rounded`, and standard HTML props listed above. The `position` and `padding` props are specific to `River.Visual`.
281+
`RiverBreakout.Visual` supports the `children`, `fillMedia`, `hasShadow`, `rounded`, and standard HTML props listed above. The `position` and `padding` props are specific to `River.Visual`.
283282

284283
### River.Content and RiverBreakout.Content <Label>Required</Label>
285284

apps/next-docs/content/components/RiverBreakoutTabs/index.mdx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,8 @@ import {RiverBreakoutTabs} from '@primer/react-brand'
166166

167167
### RiverBreakoutTabs.Visual <Label>Required</Label>
168168

169-
| Name | Type | Default | Description |
170-
| :--------------------- | :---------------------- | :-----: | :-------------------------------- |
171-
| `children` | `React.ReactElement` | | Item visual media |
172-
| `imageBackgroundColor` | `'default' \| 'subtle'` | | Optional background around visual |
169+
| Name | Type | Default | Description |
170+
| :--------- | :------------------- | :-----: | :---------------- |
171+
| `children` | `React.ReactElement` | | Item visual media |
173172

174173
`RiverBreakoutTabs.Visual` extends `River.Visual` and supports all `River.Visual` props.
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/** @jest-environment node */
2+
3+
import React from 'react'
4+
import {renderToString} from 'react-dom/server'
5+
import {useFocusTrap} from './useFocusTrap'
6+
7+
describe('useFocusTrap SSR', () => {
8+
it('renders without a document', () => {
9+
const TestComponent = () => {
10+
useFocusTrap()
11+
return null
12+
}
13+
14+
expect(() => renderToString(<TestComponent />)).not.toThrow()
15+
})
16+
})
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import React from 'react'
2+
import {focusTrap} from '@primer/behaviors'
3+
import {render, renderHook} from '@testing-library/react'
4+
import {useFocusTrap} from './useFocusTrap'
5+
6+
jest.mock('@primer/behaviors', () => ({
7+
focusTrap: jest.fn(),
8+
}))
9+
10+
const mockFocusTrap = jest.mocked(focusTrap)
11+
12+
describe('useFocusTrap', () => {
13+
const ToggleFocusTrapTestComponent = ({
14+
disabled,
15+
restoreFocusOnCleanUp = false,
16+
}: {
17+
disabled: boolean
18+
restoreFocusOnCleanUp?: boolean
19+
}) => {
20+
const {containerRef} = useFocusTrap<HTMLDivElement>({disabled, restoreFocusOnCleanUp})
21+
return <div ref={containerRef} />
22+
}
23+
24+
beforeEach(() => {
25+
mockFocusTrap.mockReturnValue(new AbortController())
26+
})
27+
28+
afterEach(() => {
29+
jest.clearAllMocks()
30+
})
31+
32+
it('creates stable refs when refs are not provided', () => {
33+
const {result, rerender} = renderHook(() => useFocusTrap({disabled: true}))
34+
const initialContainerRef = result.current.containerRef
35+
const initialFocusRef = result.current.initialFocusRef
36+
37+
expect(initialContainerRef.current).toBeNull()
38+
expect(initialFocusRef.current).toBeNull()
39+
40+
rerender()
41+
42+
expect(result.current.containerRef).toBe(initialContainerRef)
43+
expect(result.current.initialFocusRef).toBe(initialFocusRef)
44+
})
45+
46+
it('preserves provided refs', () => {
47+
const containerRef = React.createRef<HTMLDivElement>()
48+
const initialFocusRef = React.createRef<HTMLButtonElement>()
49+
const {result} = renderHook(() => useFocusTrap({containerRef, initialFocusRef, disabled: true}))
50+
51+
expect(result.current.containerRef).toBe(containerRef)
52+
expect(result.current.initialFocusRef).toBe(initialFocusRef)
53+
})
54+
55+
it('passes generated ref elements to the focus trap', () => {
56+
const TestComponent = () => {
57+
const {containerRef, initialFocusRef} = useFocusTrap<HTMLDivElement, HTMLButtonElement>()
58+
59+
return (
60+
<div ref={containerRef}>
61+
<button ref={initialFocusRef} />
62+
</div>
63+
)
64+
}
65+
66+
const {container} = render(<TestComponent />)
67+
68+
expect(mockFocusTrap).toHaveBeenCalledWith(container.querySelector('div'), container.querySelector('button'))
69+
})
70+
71+
it('starts and aborts the focus trap when disabled changes', () => {
72+
const abortController = new AbortController()
73+
const abortSpy = jest.spyOn(abortController, 'abort')
74+
mockFocusTrap.mockReturnValue(abortController)
75+
76+
const {rerender} = render(<ToggleFocusTrapTestComponent disabled />)
77+
78+
expect(mockFocusTrap).not.toHaveBeenCalled()
79+
80+
rerender(<ToggleFocusTrapTestComponent disabled={false} />)
81+
82+
expect(mockFocusTrap).toHaveBeenCalledTimes(1)
83+
84+
rerender(<ToggleFocusTrapTestComponent disabled />)
85+
86+
expect(abortSpy).toHaveBeenCalled()
87+
})
88+
89+
it('captures fresh focus after cleaning up a non-HTMLElement active element', () => {
90+
const svg = document.createElementNS('http://www.w3.org/2000/svg', 'svg')
91+
svg.setAttribute('tabindex', '0')
92+
document.body.append(svg)
93+
svg.focus()
94+
95+
const nextFocusedElement = document.createElement('button')
96+
document.body.append(nextFocusedElement)
97+
98+
const {rerender} = render(<ToggleFocusTrapTestComponent disabled={false} restoreFocusOnCleanUp />)
99+
100+
rerender(<ToggleFocusTrapTestComponent disabled restoreFocusOnCleanUp />)
101+
nextFocusedElement.focus()
102+
103+
const focusSpy = jest.spyOn(nextFocusedElement, 'focus')
104+
105+
rerender(<ToggleFocusTrapTestComponent disabled={false} restoreFocusOnCleanUp />)
106+
rerender(<ToggleFocusTrapTestComponent disabled restoreFocusOnCleanUp />)
107+
108+
expect(focusSpy).toHaveBeenCalled()
109+
110+
svg.remove()
111+
nextFocusedElement.remove()
112+
})
113+
})

0 commit comments

Comments
 (0)