Skip to content

Commit

Permalink
Setting overflow: auto without a media query to fix resizable pane bu…
Browse files Browse the repository at this point in the history
…g. (#2685)

* Setting overflow: auto without a media query to fix resizable pane bug.

* Create cuddly-brooms-shake.md

* Update src/PageLayout/PageLayout.tsx

Co-authored-by: Cole Bemis <colebemis@github.com>

* Updated snapshots.

* Add test to make sure we're not breaking resizable panes.

* Update src/PageLayout/PageLayout.tsx

Co-authored-by: Cole Bemis <colebemis@github.com>

* Updated snapshots.

Co-authored-by: Cole Bemis <colebemis@github.com>
  • Loading branch information
radglob and colebemis committed Dec 19, 2022
1 parent 5dd4bb1 commit 3a8bb76
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/cuddly-brooms-shake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Setting overflow: auto without a media query to fix resizable pane bug.
30 changes: 29 additions & 1 deletion src/PageLayout/PageLayout.test.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React from 'react'
import {act, render, screen} from '@testing-library/react'
import {act, fireEvent, render, screen} from '@testing-library/react'
import MatchMediaMock from 'jest-matchmedia-mock'
import 'react-intersection-observer/test-utils'
import {ThemeProvider} from '..'
import {viewportRanges} from '../hooks/useResponsiveValue'
import {PageLayout} from './PageLayout'
import {Placeholder} from '../Placeholder'

let matchMedia: MatchMediaMock

Expand Down Expand Up @@ -171,5 +172,32 @@ describe('PageLayout', () => {
)
expect(ref).toHaveBeenCalledWith(screen.getByTestId('content').parentNode)
})

it('should be resizable if `resizable` is set correctly', async () => {
render(
<ThemeProvider>
<PageLayout>
<PageLayout.Pane resizable>
<Placeholder height={320} label="Pane" />
</PageLayout.Pane>
<PageLayout.Content>
<Placeholder height={640} label="Content" />
</PageLayout.Content>
</PageLayout>
</ThemeProvider>,
)

const placeholder = await screen.findByText('Pane')
const pane = placeholder.parentNode
const initialWidth = (pane as HTMLElement).style.getPropertyValue('--pane-width')

const divider = await screen.findByRole('separator')
// Moving divider should resize pane.
fireEvent.mouseDown(divider)
fireEvent.mouseMove(divider)
fireEvent.mouseUp(divider)
const finalWidth = (pane as HTMLElement).style.getPropertyValue('--pane-width')
expect(finalWidth).not.toEqual(initialWidth)
})
})
})
5 changes: 2 additions & 3 deletions src/PageLayout/PageLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ const VerticalDivider: React.FC<React.PropsWithChildren<DividerProps & Draggable
bg: isDragging ? 'accent.fg' : 'neutral.muted',
},
}}
role="separator"
onMouseDown={() => {
setIsDragging(true)
onDragStart?.()
Expand Down Expand Up @@ -675,9 +676,7 @@ const Pane = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageLayout
? ['100%', null, 'clamp(var(--pane-min-width), var(--pane-width), var(--pane-max-width))']
: paneWidths[width],
padding: SPACING_MAP[padding],
[`@media screen and (min-width: ${theme.breakpoints[1]})`]: {
overflow: 'auto',
},
overflow: [null, null, 'auto'],

[`@media screen and (min-width: ${theme.breakpoints[3]})`]: {
'--pane-max-width': 'calc(100vw - 959px)',
Expand Down
4 changes: 4 additions & 0 deletions src/PageLayout/__snapshots__/PageLayout.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ exports[`PageLayout renders condensed layout 1`] = `
@media screen and (min-width:768px) {
.c11 {
width: 256px;
overflow: auto;
}
}
Expand Down Expand Up @@ -423,6 +424,7 @@ exports[`PageLayout renders default layout 1`] = `
@media screen and (min-width:768px) {
.c11 {
width: 256px;
overflow: auto;
}
}
Expand Down Expand Up @@ -715,6 +717,7 @@ exports[`PageLayout renders pane in different position when narrow 1`] = `
@media screen and (min-width:768px) {
.c11 {
width: 256px;
overflow: auto;
}
}
Expand Down Expand Up @@ -983,6 +986,7 @@ exports[`PageLayout renders with dividers 1`] = `
@media screen and (min-width:768px) {
.c10 {
width: 256px;
overflow: auto;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ exports[`SplitPageLayout renders default layout 1`] = `
@media screen and (min-width:768px) {
.c11 {
width: 256px;
overflow: auto;
}
}
Expand Down

0 comments on commit 3a8bb76

Please sign in to comment.