improve smooth module scrolling logic #12691
Conversation
|
I can confirm that it fixes the bug for me. |
|
By the way, is there a way how to disable the opening/collapsing animation of modules? For me this is an annoying gadget. |
|
Yes; set duration to 0ms in preferences |
Ups, missed this one, thanks |
It will be in the release notes. |
TurboGit
added
the
priority: high
| const gboolean scroll_to_top = dt_conf_get_bool("darkroom/ui/scroll_to_module"); | ||
| const int spare = available.height - allocation.height; | ||
| const int from_top = allocation.y - value; | ||
| const int move = MAX(from_top - scroll_to_top ? 0 : MAX(0, MIN(from_top, spare)), |
There was a problem hiding this comment.
@TurboGit I'm scratching my head here. This functionality has stopped working for me, in that it doesn't scroll up a module anymore when I extend it out of view at the bottom or when "scroll to top" is enabled. I believe that I extensively tested this before I submitted it to you and you did as well, but somehow now the behaviour seems to have changed. Even in my local branch.
Looking at the code, I think that's because there should be brackets around
scroll_to_top ? 0 : MAX(0, MIN(from_top, spare))
but then I don't understand why it ever worked. Really mystified. Maybe I compiled with a different (buggy?) compiler (gcc/llvm) or version before? But I also tested on windows/MSYS2 where it worked before and now doesn't and I haven't changed anything there.
Does it still work for you? Should I submit a new fix? After more testing? I'm just quite confused.
There was a problem hiding this comment.
Does it still work for you? Should I submit a new fix? After more testing? I'm just quite confused.
No it doesn't work for me either. I'm not sure I have tested this scenario though. I was looking at the fact that it should not scroll when I use the blend controls, but I've probably done that on a small module that does not extends much on the bottom.
There was a problem hiding this comment.
But does it "scroll to top" when expanding a module? (if you have that option selected in prefs) For me it basically doesn't scroll ever...
It's not that it surprises me that the code as is doesn't work. It is that I recall testing it extensively and (obviously) seeing it working, before submitting the PR. Maybe I was tired or something and made a last minute change, but I should have immediately caught this.
And I'm also surprised not to see any complaints that scroll-to-top stopped working (whereas there are complaints that smoothly expanding is annoying).
There was a problem hiding this comment.
Seems I "simplified" from
const int move = MAX(scroll_to_top ? from_top : from_top - MAX(0, MIN(from_top, spare)), - MAX(0, spare - from_top));
at the last minute, without realising that's not the same thing (and doing sloppy testing). Apologies.
But now I also identified two more corner cases:
- when expanding a module below the currently open (and therefore now collapsing) module, I'm trying to keep it in the same place. But afterwards, the user can move it. If it then changes size, it again will be brought back to where it was when it was expanded. This should not happen.
- both left (lib) and right (iop) modules can be moving at the same time (for example because the history size changed). This can cause modules to move (upward) when they should be staying in the same place.
I'm working on solutions for all three.
I believe this fixes #12686 but please test all scenarios carefully because there might still be corner cases I haven't thought of.
I haven't found a way to prevent the resize and repositioning triggered by hovering over the notebook tabs or manipulating the channel controls. So whenever a resize happens, there might still be a move. What this does try to do is avoid scrolling any part of the active module (further) out of view. It will still try to show as much as possible of the module if anything was hidden that there is (now) space for or, in case "scroll to top" is on, move the module to the top again if that doesn't obscure anything currently visible. Hopefully some of the "unnecessary" scrolling is not too annoying and maybe even helpful.
For me, switching off "scroll to top" gives a better experience overall than having it on, as these changes make it (imvho of course) no longer that useful.