Skip to content

Commit 42b7ddc

Browse files
committed
Fix layout bug
1 parent 4f2918c commit 42b7ddc

File tree

4 files changed

+55
-12
lines changed

4 files changed

+55
-12
lines changed

packages/react/src/PageLayout/PageLayout.module.css

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
paneMaxWidthDiffBreakpoint: 1280;
55
/* Default value for --pane-max-width-diff below the breakpoint */
66
paneMaxWidthDiffDefault: 511;
7+
/* Default value for --sidebar-max-width-diff (constant across all viewports) */
8+
sidebarMaxWidthDiffDefault: 256;
79
}
810

911
.PageLayoutRoot {
@@ -29,6 +31,8 @@
2931
--pane-width-large: 100%;
3032
/* NOTE: This value is exported via :export for use in usePaneWidth.ts */
3133
--pane-max-width-diff: 511px;
34+
/* Sidebar uses a smaller diff since it doesn't need to reserve space for a file tree pane */
35+
--sidebar-max-width-diff: 256px;
3236

3337
@media screen and (min-width: 768px) {
3438
--pane-width-small: 240px;
@@ -762,7 +766,7 @@
762766
*/
763767
display: flex;
764768
flex-direction: row;
765-
flex-shrink: 0;
769+
flex-shrink: 1;
766770
height: 100%;
767771

768772
&:where([data-is-hidden='true']) {

packages/react/src/PageLayout/PageLayout.test.tsx

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,4 +272,23 @@ describe('PageLayout', async () => {
272272
expect(container.firstChild?.nodeName).toEqual('DIV')
273273
})
274274
})
275+
276+
describe('PageLayout.Sidebar', () => {
277+
it('SidebarWrapper should allow shrinking to prevent overflow at narrow viewports', () => {
278+
const {container} = render(
279+
<PageLayout>
280+
<PageLayout.Content>Content</PageLayout.Content>
281+
<PageLayout.Sidebar resizable width={{min: '256px', default: '296px', max: '768px'}}>
282+
Sidebar
283+
</PageLayout.Sidebar>
284+
</PageLayout>,
285+
)
286+
287+
const sidebarWrapper = container.querySelector<HTMLElement>('[class*="SidebarWrapper"]')
288+
expect(sidebarWrapper).not.toBeNull()
289+
290+
const style = getComputedStyle(sidebarWrapper!)
291+
expect(style.flexShrink).toBe('1')
292+
})
293+
})
275294
})

packages/react/src/PageLayout/usePaneWidth.test.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
updateAriaValues,
1010
defaultPaneWidth,
1111
DEFAULT_MAX_WIDTH_DIFF,
12+
DEFAULT_SIDEBAR_MAX_WIDTH_DIFF,
1213
SSR_DEFAULT_MAX_WIDTH,
1314
ARROW_KEY_STEP,
1415
} from './usePaneWidth'
@@ -1141,14 +1142,23 @@ describe('helper functions', () => {
11411142
})
11421143

11431144
describe('getPaneMaxWidthDiff', () => {
1144-
it('should return default when element is null', () => {
1145+
it('should return default pane diff when element is null', () => {
11451146
expect(getPaneMaxWidthDiff(null)).toBe(DEFAULT_MAX_WIDTH_DIFF)
11461147
})
11471148

1148-
it('should return default when CSS variable is not set', () => {
1149+
it('should return default sidebar diff when element is null and isSidebar is true', () => {
1150+
expect(getPaneMaxWidthDiff(null, true)).toBe(DEFAULT_SIDEBAR_MAX_WIDTH_DIFF)
1151+
})
1152+
1153+
it('should return default pane diff when CSS variable is not set', () => {
11491154
const element = document.createElement('div')
11501155
expect(getPaneMaxWidthDiff(element)).toBe(DEFAULT_MAX_WIDTH_DIFF)
11511156
})
1157+
1158+
it('should return default sidebar diff when CSS variable is not set and isSidebar is true', () => {
1159+
const element = document.createElement('div')
1160+
expect(getPaneMaxWidthDiff(element, true)).toBe(DEFAULT_SIDEBAR_MAX_WIDTH_DIFF)
1161+
})
11521162
})
11531163

11541164
describe('updateAriaValues', () => {
@@ -1187,6 +1197,7 @@ describe('helper functions', () => {
11871197
describe('constants', () => {
11881198
it('should export expected constants', () => {
11891199
expect(DEFAULT_MAX_WIDTH_DIFF).toBe(511)
1200+
expect(DEFAULT_SIDEBAR_MAX_WIDTH_DIFF).toBe(256)
11901201
expect(SSR_DEFAULT_MAX_WIDTH).toBe(600)
11911202
expect(ARROW_KEY_STEP).toBe(3)
11921203
expect(defaultPaneWidth).toEqual({small: 256, medium: 296, large: 320})

packages/react/src/PageLayout/usePaneWidth.ts

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,12 @@ export type UsePaneWidthResult = {
6868
*/
6969
export const DEFAULT_MAX_WIDTH_DIFF = Number(cssExports.paneMaxWidthDiffDefault)
7070

71+
/**
72+
* Default value for --sidebar-max-width-diff CSS variable.
73+
* Unlike --pane-max-width-diff, this is constant across all viewport sizes.
74+
*/
75+
export const DEFAULT_SIDEBAR_MAX_WIDTH_DIFF = Number(cssExports.sidebarMaxWidthDiffDefault)
76+
7177
// --pane-max-width-diff changes at this breakpoint in PageLayout.module.css.
7278
const DEFAULT_PANE_MAX_WIDTH_DIFF_BREAKPOINT = Number(cssExports.paneMaxWidthDiffBreakpoint)
7379
/**
@@ -106,14 +112,17 @@ export const getDefaultPaneWidth = (w: PaneWidthValue): number => {
106112
}
107113

108114
/**
109-
* Gets the --pane-max-width-diff CSS variable value from a pane element.
110-
* This value is set by CSS media queries and controls the max pane width constraint.
115+
* Gets the max-width-diff CSS variable value from a pane element.
116+
* For sidebars, reads --sidebar-max-width-diff (constant across viewports).
117+
* For panes, reads --pane-max-width-diff (changes at 1280px breakpoint).
111118
* Note: This calls getComputedStyle which forces layout - cache the result when possible.
112119
*/
113-
export function getPaneMaxWidthDiff(paneElement: HTMLElement | null): number {
114-
if (!paneElement) return DEFAULT_MAX_WIDTH_DIFF
115-
const value = parseInt(getComputedStyle(paneElement).getPropertyValue('--pane-max-width-diff'), 10)
116-
return value > 0 ? value : DEFAULT_MAX_WIDTH_DIFF
120+
export function getPaneMaxWidthDiff(paneElement: HTMLElement | null, isSidebar = false): number {
121+
const defaultValue = isSidebar ? DEFAULT_SIDEBAR_MAX_WIDTH_DIFF : DEFAULT_MAX_WIDTH_DIFF
122+
const cssVar = isSidebar ? '--sidebar-max-width-diff' : '--pane-max-width-diff'
123+
if (!paneElement) return defaultValue
124+
const value = parseInt(getComputedStyle(paneElement).getPropertyValue(cssVar), 10)
125+
return value > 0 ? value : defaultValue
117126
}
118127

119128
// Helper to update ARIA slider attributes via direct DOM manipulation
@@ -197,7 +206,7 @@ export function usePaneWidth({
197206
})
198207
// Cache the CSS variable value to avoid getComputedStyle during drag (causes layout thrashing)
199208
// Updated on mount and resize when breakpoints might change
200-
const maxWidthDiffRef = React.useRef(DEFAULT_MAX_WIDTH_DIFF)
209+
const maxWidthDiffRef = React.useRef(constrainToViewport ? DEFAULT_SIDEBAR_MAX_WIDTH_DIFF : DEFAULT_MAX_WIDTH_DIFF)
201210

202211
// Calculate max width constraint - for custom widths this is capped to viewport bounds
203212
// when constrainToViewport is set (e.g., Sidebar), otherwise it uses the custom max directly.
@@ -337,7 +346,7 @@ export function usePaneWidth({
337346
lastViewportWidth = currentViewportWidth
338347

339348
if (crossedBreakpoint) {
340-
maxWidthDiffRef.current = getPaneMaxWidthDiff(paneRef.current)
349+
maxWidthDiffRef.current = getPaneMaxWidthDiff(paneRef.current, constrainToViewport)
341350
}
342351

343352
const actualMax = getMaxPaneWidthRef.current()
@@ -365,7 +374,7 @@ export function usePaneWidth({
365374
}
366375

367376
// Initial calculation on mount
368-
maxWidthDiffRef.current = getPaneMaxWidthDiff(paneRef.current)
377+
maxWidthDiffRef.current = getPaneMaxWidthDiff(paneRef.current, constrainToViewport)
369378
const initialMax = getMaxPaneWidthRef.current()
370379
setMaxPaneWidth(initialMax)
371380
paneRef.current?.style.setProperty('--pane-max-width', `${initialMax}px`)

0 commit comments

Comments
 (0)