-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Desktop: Resolves #10151 Implement Proportional Panel Sizing on Window Resize #13480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
…es size so splits stay consistent instead of stretching only the rightmost pane. - Scale stored item dimensions according to the parent container's direction, clamp with existing minimums, and re-run validation to keep resize handles correct (packages/app-desktop/gui/ResizableLayout/utils/scaleLayoutItemSizes.ts:1). - Compute width/height ratios during updateRootLayoutSize, fall back gracefully if the previous size is unavailable, and apply the scaling helper before dispatching the new layout state (packages/app-desktop/gui/MainScreen.tsx:315).
As noted in the last video in PR laurent22#10151 comments, if you resize panels after using change application layout it can result in multiple panels being resized. This is a pre-existing issue that happens in current joplin releases. - Modified onResizeStart handler in the renderLayoutItem function - Before recording the resized item, we iterate through all siblings - For any sibling without an explicit size, we set it to its current calculated size - This "freezes" all other panels during the resize operation
I often switch between different resolutions and DPI scaling. Is this also going to work properly with that? What I mean is switching from a small laptop screen with 250% scaling to a large monitor with 125% scaling, or just moving the Joplin window between the two screens, etc. |
Issue: when dev console is automatically opened on launch, if you then closed the dev tools it would result in panel sizes being larger than they were when you last closed joplin. This issue exposed a missing piece in the resize logic: we only scaled panels when a resize event happened after the renderer was already running. When you reopen in dev mode the renderer starts with a narrower content width (devtools docked), but the saved layout still carries the wider pixel widths from the previous session. Because we never adjusted those widths before the first "grow" event (closing devtools), the next resize multiplied already-wide values and the columns crept wider each launch. Fix: - Keep track of the root size that was in effect when the layout was saved (packages/app-desktop/gui/ResizableLayout/utils/persist.ts:18). When reloading, compare that snapshot with the current window size and apply the same proportional scaling we use during live resizes (packages/app-desktop/gui/ResizableLayout/utils/persist.ts:55). - Update the scaling helper so the root context always records the latest window dimensions, ensuring future loads have a correct baseline (packages/app-desktop/gui/ResizableLayout/utils/scaleLayoutItemSizes.ts:20). With this, layouts now normalise immediately on startup, so repeatedly launching in dev mode and closing the devtools leaves the panels the same size.
…sible size adjusted the scaler to remember each panel's natural width/height (the value before any min-size clamping) in context.autoSize, and reuse that untouched baseline for future resizes. The helper now: - Tracks and restores the root size snapshot while leaving existing context data intact (packages/app-desktop/gui/ResizableLayout/utils/scaleLayoutItemSizes.ts:20). - Stores per-item natural widths/heights so clamp enforcement no longer corrupts later ratios, and cleans the metadata when it's not needed (packages/app-desktop/gui/ResizableLayout/utils/scaleLayoutItemSizes.ts:41). - Strips this transient autoSize field when persisting layouts so saved data stays compact (packages/app-desktop/gui/ResizableLayout/utils/persist.ts:34).
Window resizing was reverting panels because the proportional scaler still held onto the *previous* natural widths from before you dragged a splitter. When a manual resize happened we updated item.width, but the scaler's cached baseline stayed stale, so the next window resize snapped the panel back to that old value. Now, whenever setLayoutItemProps applies a width/height change (e.g., during a drag): - It records the new explicit size as the panel's natural width/height in context.autoSize, so the scaler uses the latest user-set baseline. - If a size gets cleared, the helper drops the corresponding natural value and prunes the helper context so nothing leaks into persistence.
- packages/app-desktop/gui/ResizableLayout/utils/scaleLayoutItemSizes.ts:16 now tracks naturalWidth/naturalHeight together with the root dimensions they were measured against, and recomputes panel sizes relative to that snapshot instead of mutating the snapshot on each window resize. The root’s savedRootSize is no longer touched during transient resizes, so the next session still scales from the layout you saved. - packages/app-desktop/gui/ResizableLayout/utils/setLayoutItemProps.ts:5 updates the helper context whenever you drag a splitter, recording both the new size and the current root dimensions, and cleans up the metadata when a size is cleared. That gives manual adjustments a fresh baseline without leaking anything into the persisted layout.
1. Created autoSizeUtils.ts - A new shared utility file containing: - AutoSizeContext interface (now imported from types.ts) - ensureContext() - Ensures the context object exists - ensureAutoSizeContext() - Ensures the autoSize context exists - currentAutoSizeContext() - Gets the current autoSize context or null - cleanupAutoSizeContext() - Cleans up empty autoSize context 2. Updated scaleLayoutItemSizes.ts: - Removed duplicate AutoSizeContext interface - Removed duplicate helper functions (ensureContext, ensureAutoSizeContext, currentAutoSize, cleanupAutoSizeContext) - Now imports and uses helpers from autoSizeUtils.ts - Fixed function call from currentAutoSize to currentAutoSizeContext for consistency 3. Updated setLayoutItemProps.ts: - Removed duplicate AutoSizeContext interface - Removed duplicate helper functions (ensureContext, ensureAutoSize, cleanupAutoSize) - Now imports and uses helpers from autoSizeUtils.ts 4. Updated types.ts: - Added AutoSizeContext interface (single source of truth) - Added LayoutItemContext interface - Properly typed LayoutItem.context property
I don't currently have another monitor to test the scenario with dragging across monitors, but I tested changing windows scaling settings and didn't see any issues On paper I wouldn't expect any problems since everything should scale proportionally scaling.mp4EDIT: I was able to hook my laptop up to a display and tested dragging between display 1 (2880 resolution, 200% scaling) and display 2 (4k resolution, 150% scaling) and didn't see any issues, but not sure if there's a good way to record a video of that. It basically behaves the same as in the above video where I was changing the scaling setting. |
I've fixed all edge cases that I encountered in testing, though the changes ended up being more extensive than I'd like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this! I've looked through some of the changes and left comments.
if (!currentWidth || !currentHeight) { | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied | ||
this.updateMainLayout(produce(layout, (draft: any) => { | ||
draft.width = newSize.width; | ||
draft.height = newSize.height; | ||
})); | ||
return; | ||
} | ||
|
||
const widthRatio = newSize.width / currentWidth; | ||
const heightRatio = newSize.height / currentHeight; | ||
|
||
this.updateMainLayout(scaleLayoutItemSizes(layout, newSize, widthRatio, heightRatio)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!currentWidth || !currentHeight) { | |
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied | |
this.updateMainLayout(produce(layout, (draft: any) => { | |
draft.width = newSize.width; | |
draft.height = newSize.height; | |
})); | |
return; | |
} | |
const widthRatio = newSize.width / currentWidth; | |
const heightRatio = newSize.height / currentHeight; | |
this.updateMainLayout(scaleLayoutItemSizes(layout, newSize, widthRatio, heightRatio)); | |
if (!currentWidth || !currentHeight) { | |
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied | |
this.updateMainLayout(produce(layout, (draft: any) => { | |
draft.width = newSize.width; | |
draft.height = newSize.height; | |
})); | |
} else { | |
const widthRatio = newSize.width / currentWidth; | |
const heightRatio = newSize.height / currentHeight; | |
this.updateMainLayout(scaleLayoutItemSizes(layout, newSize, widthRatio, heightRatio)); | |
} |
Optional/style: Consider rewriting this as an if
/else
(removing the early return).
function clamp(value: number, minimum: number) { | ||
return Math.max(minimum, Math.round(value)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using a different name here to avoid confusion. (There's a JavaScript proposal for adding a Math.clamp
function and a CSS clamp
function, both of which behave differently from the above clamp
.)
const referenceRootWidth = ratioX ? newRootSize.width / ratioX : undefined; | ||
const referenceRootHeight = ratioY ? newRootSize.height / ratioY : undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: Currently, ratioX
should be non-zero (due to the definition of safeRatio
). As such, it shouldn't be possible for !ratioX === true
or !ratioY === true
. As a result, it should be safe to simplify the above to:
const referenceRootWidth = newRootSize.width / ratioX;
const referenceRootHeight = newRootSize.height / ratioY;
|
||
// Calculate natural width (before min-size clamping) if not already tracked | ||
if (typeof ensuredAutoSize.naturalWidth !== 'number' && hasExplicitWidth) { | ||
const targetReferenceWidth = ensuredAutoSize.rootWidth ?? referenceRootWidth ?? savedRootWidth ?? previousRootWidth ?? newRootSize.width; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const targetReferenceWidth = ensuredAutoSize.rootWidth ?? referenceRootWidth ?? savedRootWidth ?? previousRootWidth ?? newRootSize.width; | |
const targetReferenceWidth = ensuredAutoSize.rootWidth ?? referenceRootWidth; |
referenceRootWidth
is never undefined
(see comment above).
const sourceReferenceWidth = referenceRootWidth ?? previousRootWidth ?? targetReferenceWidth; | ||
const widthValue = item.width; | ||
|
||
if (typeof targetReferenceWidth === 'number' && typeof sourceReferenceWidth === 'number' && sourceReferenceWidth > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above, since targetReferenceWidth
falls back to referenceRootWidth
, targetReferenceWidth
should always be a number. Similarly, sourceReferenceWidth
should also always be a number. As such, it should be possible to simplify this to:
if (sourceReferenceWidth > 0) {
- safeRatio() returns 1 if the value is not finite or <= 0 - ratioX and ratioY will always be positive numbers (at minimum, they'll be 1)
1. safeRatio() guarantees positive numbers - So ratioX and ratioY are always >= 1 2. referenceRootWidth/Height are always numbers - No need for nullish coalescing 3. ensuredAutoSize.rootWidth/Height are set immediately - No need for fallback chains 4. Removed unused variables - savedRootWidth, savedRootHeight, previousRootWidth, previousRootHeight were only needed for the fallback chains
No current dimensions > Just set the new size Has current dimensions > Calculate ratios and apply proportional scaling
I've committed changes that should address the above comments Some potential room for improvement after spending more time with the updated resize functionality:
|
In that case, what would happen if the user deliberately made those very thin (e.g. on a small screen, to provide more space for the note editor)? |
I didn't test yet and didn't read the whole thread, but I don't think proportional scaling is what we want. We want all panels to stay at the same width, and the editor panel to be resized, wherever it is. Proportional scaling is just maybe a little bit better but not the proper fix unfortunately. Edit: that was in fact mentioned in the linked issue:
|
Implement Proportional Panel Sizing on Window Resize
Summary
This PR implements proportional panel sizing when the Joplin window is resized. Previously, only the rightmost panel would dynamically resize when the window size changed, which was problematic when custom layouts placed plugin panels to the right of the editor.
Problem
Before: When resizing the Joplin window, only the rightmost panel (the last panel without an explicit width) would grow or shrink to fill the available space. All other panels maintained their fixed pixel widths.
Issue: This caused poor UX when users customized their layouts. For example, in a layout like
Notebooks > Editor > Outline Plugin
, resizing the window would only resize the Outline panel, leaving the editor at a fixed width even when more space was available.See: #10151
Solution
This PR implements a proportional scaling approach where all panels maintain their relative sizes when the window is resized.
Implementation:
MainScreen.updateRootLayoutSize()
to calculate width/height ratios and apply proportional scalingExample:
All panels grow proportionally, maintaining the user's intended layout balance.
Additional Fix: Panel Resize Behavior
This PR also fixes a separate issue where resizing a panel in certain custom layouts would cause neighboring panels to resize unexpectedly from both sides, which was noted in the last video in the comments from this previous PR: #10298
Problem: When multiple panels lacked explicit widths, resizing one panel would cause all flexible panels to be recalculated, resulting in panels resizing from both left and right edges simultaneously.
Solution: Modified onResizeStart in ResizableLayout.tsx to fix the sizes of all sibling panels that don't have explicit dimensions when a resize operation begins. This ensures only the panel being dragged and the last flexible panel change size.
Edge Case Fixes
The implementation includes solutions for several edge cases:
Dev mode console behavior: When Joplin launches in dev mode with the console open, closing the console no longer causes panel sizes to grow unexpectedly. This is handled by saving the root window size with the layout and applying proportional scaling on load.
Minimum size constraints: When the window is resized to its minimum size and then expanded again, panels maintain their correct proportions. This is achieved by tracking "natural" sizes (before minimum constraints are applied) separately from the actual rendered sizes.
Manual panel resize + window resize: After manually resizing a panel by dragging, subsequent window resizes correctly scale from the new manually-set size rather than reverting to a previous cached value.
Files Changed
New Files
Modified Files
Testing
Demo2-compressed.mp4
Backwards Compatibility
Fully backward compatible. Existing layouts are preserved, and the changes only affect how panels resize when the window size changes. The auto-size tracking context is transient and cleaned up automatically, so it doesn't bloat persisted layouts.