Adding a Zoom option for scrolling in the configuration
BUG: 254511
FIXED-IN: 18.12.0
ngraham |
Gwenview |
Adding a Zoom option for scrolling in the configuration
BUG: 254511
FIXED-IN: 18.12.0
Test scrolling in image and folder view for each configuration selection.
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
Wow, this works great! Did you use Qt Creator to change the .ui file, though? The diff is very large for such a small UI change. I'd prefer a hand-edited file so that the change is manageable to review.
That said, it all seems to work great, with both a mouse wheel and a touchpad. The zoom speed feels good. Code change seems sane to me.
Other Gwenview folks, whaddaya think?
Regardless, we need a few formatting changes to the diff here on Phabricator:
BUG: 254511 FIXED-IN: 18.12.0
For information regarding why, see https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch
How do I push my latest commit to the remote branch? It seems to be replacing all my old commits with the most recent.
I tried to cleanup the imageviewconfigpage.ui but it only seemed to have replaced my diff, not added my latest commit to it.
Alright, I built a new branch and just commited all my changes to that, diff seems to look good
Looks good to me and scrolling with Shift/Alt+wheel is still possible.
Just one thing I noticed - see inline comment.
lib/documentview/documentview.cpp | ||
---|---|---|
735โ736 | If MouseWheelBehavior::Zoom is active the zoom part is called no matter what canZoom() returns (e.g. for videos this is false). I can't see any problem ATM but maybe we should respect canZoom(). |