Desktop: Fixes panels overflowing window

[mablin7]

Fixes #4864 (specifically the issue raised in this comment, the original seems to have been fixed already).

The problem was that the layout logic had no way to handle maximum sizes, so if a panel was too big it simply overflowed and could even push other panes completely off-screen (see recordings below).

My fix is to add a extra logic to useLayoutItemSizes(), which

1. keeps track of the minimum space that should be reserved, such that all panels are visible (using the minWidth/minHeight props or 40px per pane if not defined)
2. if the panes with definite size don't leave enough room, makes the largest one smaller
3. calculates a maximum possible size for each pane

The calculated max size is then passed to the Resizable component, so that it's enforced during resizing. I thought the cleanest way to do this is to extend the sizes object, which keeps the calculated size of each item. It now also contains their minimum and maximum sizes.

I included a few unit tests which hopefully cover all the new functionality.

Steps to check manually

Resizing with the mouse

1. Open Joplin
2. Resize the note list, as wide as it goes
   Without the fix, the note editor is pushed off-screen. With the fix, it should stop when the note editor becomes only 40px wide.

Resizing the window

1. Open Joplin and maximize it
2. Resize the note list, really wide
3. Unmaximize Joplin
   Without the fix, the note editor suddenly becomes invisible. With the fix, it should be visible and 40px wide (its minimum size).

Screen recordings

Before

After

[laurent22]

Thanks for the pull request. The code is well written but I'm not entirely sure the logic to set the min/max is ideal. For me, an item can have a min width or height - either it's explicitly set, or it's default. But here you are dynamically setting a min and max every time the component is resized, which doesn't feel like the right approach.

So I'm wondering, would it be possible to get it working without creating this new current/min/max interface? If not, why is that? I guess if I could understand that part, the rest would make more sense too.

[laurent22]

I don't quite understand the logic here. The max total width of these two columns is 200, which means there's no more room for col3 while, based on the code, it should have a min width of 40. Could you clarify please (and if the test is correct, maybe add a comment)?

[laurent22]

I would expect that the math is col1.max + col2.max + col3.min = 200, but in your test it would 220. So my math doesn't add up here either. I'm suspecting I'm just not getting it but if you could clarify that would be great.

[mablin7]

Thanks for the pull request. The code is well written but I'm not entirely sure the logic to set the min/max is ideal. For me, an item can have a min width or height - either it's explicitly set, or it's default. But here you are dynamically setting a min and max every time the component is resized, which doesn't feel like the right approach.

So I'm wondering, would it be possible to get it working without creating this new current/min/max interface? If not, why is that? I guess if I could understand that part, the rest would make more sense too.

Well yeah in hindsight I can see the way I used max size is pretty ambiguous, because it doesn't work like in css for eg. The idea was that max.width gives the total available width for a particular item, so that when it's being resized the Resizable component knows where to stop. It's just the item's width + all the remaining width in the container. That's why the max widths need not add up in the test:

joplin/packages/app-desktop/gui/ResizableLayout/utils/useLayoutItemSizes.test.ts

Lines 145 to 157 in eb592a5

	children: [
	{
	key: 'col1',
	width: 50,
	},
	{
	key: 'col2',
	width: 70,
	},
	{
	key: 'col3',
	},
	],

If col1 is being resized, it can be expanded all the way up to 90px and then there would be exactly 40px left for col3:
col1 90px + col2 70px + col3 40px = 200px
After col1 is resized the max widths are updated, so col2's max width would become it's actual width, 70px, since there's no more free space. I swear it made sense when I wrote it. 😥

Now it occurred to me that the max size is only required for the item that's being currently resized. So probably the code in calculateChildrenSizes for calculating it could be moved to a new function, which can be called from ResizableLayout when the resize begins.

Do you think that's a better approach?

[mablin7]

I moved the code for calculating the maximum available size to a function, calculateMaxSizeAvailableForItem(), instead of storing it for each item, which I know realize was pretty unclear.

This function is only needed so that I can pass a maxWidth/maxHeight to Resizable, so it stops when an item would become too large and would push others off-screen. It kind of works without it, since useLayoutItemSizes then would anyway make the item smaller to fit, but I found that to be quite jerky (probably because there's a delay in recomputing the layout), while passing maxWidth works very smoothly.

The only small issue is that for this new function a bit of code is replicated from useLayoutItemSizes, but I just couldn't figure out how to avoid that, and it's really only 4 lines.

[laurent22]

Thanks for the update @mablin7, I feel the logic is clearer now. I've added a few comments but nothing tricky I think.

[mablin7]

Thanks for the feedback! Hope the tests are clearer now as well.

[laurent22]

Looks good now, thanks @mablin7!