Fix initial splitter sizes

[guihkx]

• Use correct splitter index for notes panel: Index 0 currently refers to the folders panel, while index 1 refers to the notes panel.

• Compute layout margins when resizing splitters

• Use default minimum width for initial panel sizes: These are the current, minimum panel widths set in mainwindow.ui, so let's use them as our initial panel sizes too.

• Adjust initial splitter sizes in updateFrame(): The old code didn't take into account the width of the folders panel, so the initial width of the folders panel was incorrect.

• Remove dead code to adjust splitter sizes: This should now be done by updateFrame() upon resizing the window.

Fixes #480

[nuttyartist]

Seems to work well, but in my opinion, the notes pane is just a little bit too big. Can we set it to the minimum size?

[zjeffer]

This how it looks for me on a 2560x1600 screen, after removing the splitterSizes line from the config. The two left panes look small enough for me. Is this the same thing you're seeing? @nuttyartist

[nuttyartist]

That's how it looks on my machine:

[zjeffer]

Ah, I see it can indeed be a bit smaller. I'll try to find why it doesn't use the minimum, but the code kind of implies it already uses the minimum width :thinking:

[zjeffer]

Something else I noticed: resizing the first splitter will both move and resize the note list, is this intended behaviour?

https://user-images.githubusercontent.com/4633209/219902623-a1fe1ed9-b42e-4c2a-9e14-2950dc443bef.mp4

Personally, I would expect that when resizing the first splitter, the middle pane would always stay the same size, but would be moved to the right when making the first pane larger, and move back to the left when making the first pane smaller again.

[guihkx]

That's how it looks on my machine:

Nice catch. I think I fixed it, can you please re-check?

@zjeffer Hmmm... I'm a bit on the fence on that one. Either behavior would be fine to me, I guess... I'll leave that for @nuttyartist to decide... xD

[guihkx]

I'll take a look at the failing CI build on Linux in a moment...

[guihkx]

Rebased to fix typos.

[nuttyartist]

Personally, I would expect that when resizing the first splitter, the middle pane would always stay the same size, but would be moved to the right when making the first pane larger, and move back to the left when making the first pane smaller again.

@zjeffer I'm with you on this, I think if a user changes the folders pane size he doesn't expect the other panes to resize as well.

Nice catch. I think I fixed it, can you please re-check?

Works now (:

Can you address @zjeffer comment?

[guihkx]

Works now (:

Nice. Thanks for testing!

Can you address @zjeffer comment?

I can try... :p

But not just right now... (it's kind of late in the morning and I'm tired xD)

@zjeffer (and others) Feel free to give it a shot in the meantime, though.

[zjeffer]

Nice catch. I think I fixed it, can you please re-check?

This still happens for me when starting from a clean config. I'll see what I can do.

EDIT: The problem seems to be here: https://github.com/guihkx/notes/blob/fix-initial-splitter-sizes/src/mainwindow.cpp#L1261

I put some std::couts before and after this line to check the sizes() of m_splitter:

• Before restoring the splitter state, the splitter size is correct (the minimum: 185 pixels).
• After restoring the state, the splitter size of the middle pane is too big.

If I comment this line: https://github.com/guihkx/notes/blob/fix-initial-splitter-sizes/src/mainwindow.cpp#L1095, everything seems to work as it should. I don't understand why, though. If I print the splitter size before that line, it is the correct splitter size. So for some reason, saving or restoring the splitter size will set the wrong values?

[guihkx]

@zjeffer Oh, that's interesting... Could you reproduce it using the AppImage? Just to be sure it's not a theming issue.

By the way, I tried figuring out how to implement https://github.com/nuttyartist/notes/pull/481#issuecomment-1435785578, but I didn't succeed. :S

We should probably convert that into a separate issue, since this one is trying to fix a specific problem.

[zjeffer]

Could you reproduce it using the AppImage? Just to be sure it's not a theming issue.

I tested it in a Ubuntu 22.10 VM like this:

https://user-images.githubusercontent.com/4633209/219963330-6cc4ed6f-7969-485a-ade6-b642491f4102.mp4

We should probably convert that into a separate issue, since this one is trying to fix a specific problem.

Sure, I agree.

[zjeffer]

Hmm, I just noticed it does work when I completely remove the settings.ini contents... Does the windowGeometry setting have an impact on this?

[guihkx]

Does the windowGeometry setting have an impact on this?

I think that's a possibility... I'll try to investigate it too.

[zjeffer]

Looks like it does. Deleting both splitterSizes and windowGeometry makes it work as expected.

[zjeffer]

I guess this can be merged as well, now?

[guihkx]

I think so.

I also discovered some other small bugs while investigating this, but I should probably open separate PR to make things easier for people reviewing it...

Same for the windowGeometry issue (which I haven't figured out yet).

@nuttyartist Can I merge this?

[nuttyartist]

Yes! I will merge this now and start testing, please open the issues.

[guihkx]

I noticed that all commits were squashed and merged into a single one. But in next PRs, I think it's preferable to merge commits as-is, because IMO it makes things easier when git bisecting for regressions, for example... :)

[nuttyartist]

Whoopsy, thanks for letting me know! I'll next time :)