Review Request 119549: Allow different ways to handle zoom and position between images

[Aurélien Gâteau]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119549/#review63738
-----------------------------------------------------------


Hi, looks great, thanks for the patch!

I only have minor requests for the configuration dialog:
- I suggest renaming "Shared zoom and position" to "Keep zoom and position" (and rename the enum value to Keep) and "Individual (per image) zoom and position" to "Per image zoom and position".
- Move the "Zoom mode" block above the "Enlarge smaller images" checkbox to keep all zooming options together

- Aurélien Gâteau


On juil. 31, 2014, 4:40 matin, John Zaitseff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119549/
> -----------------------------------------------------------
> 
> (Updated juil. 31, 2014, 4:40 matin)
> 
> 
> Review request for Gwenview.
> 
> 
> Bugs: 337262
>     http://bugs.kde.org/show_bug.cgi?id=337262
> 
> 
> Repository: gwenview
> 
> 
> Description
> -------
> 
> (Taken from bug #337262 )
> 
> Over the years, there has been much discussion about whether the zoom and position should be kept the same between images (see, for example, bug 293103, which I submitted, or bugs 291759, 294915, 321122, 327889, 331412, 334530, 337037 --- there may be more!).  I have even submitted a patch (with bug 293103) which was applied in modified form---thanks!  However, it has been subsequently broken...
> 
> I have come to realise that there are THREE main ways people like to have the zoom and position settings applied to successive images:
> 
> 1. Each image should be opened in Zoom to Fit mode, even if the previous image was zoomed in or out and was panned to a different position.  This is Aurélien Gâteau's preferred mode of operation: call it Autofit zoom mode.
> 
> 2. The zoom and position settings should be shared across all images.  New images should be opened with the previous image's settings.  If you go back to a previous image (from image B to image A, previously opened), image A's settings should be updated to be those of image B.  This allows you to set zoom and position on an image, then go back and forward to the previous and next image while retaining the settings.  This is MY preferred mode of operation: call it Shared mode.
> 
> 3. Each image should remember its own zoom and position settings.  New images should be opened with the previous image's position and zoom, but if you then change image B's zoom/position, this is NOT passed back to image A, if you go back to that image.  This is Abhinav Gangwar's preferred mode of operation, and is what LockZoom implements in Gwenview: call it Individual mode.
> 
> I think people don't mind Autofit being the default, as long as it can be changed.  At the moment, doing so is very non-obvious, and the Shared mode of operation simply does not exist (as of 12th February 2014).
> 
> I am submitting a patch that does the following:
> 
> 1. Create a config file option "ZoomMode" with ZoomMode::Autofit, ZoomMode::Shared and ZoomMode::Individual being the choices (Autofit is the default, per Aurélien's preferences).
> 
> 2. Remove the now-obsolete ShowLockZoomButton and LockZoom config options.
> 
> 3. Remove the Lock Zoom button: it is not visible in Autofit mode anyway, and when a user wants the other modes, they want it on ALL the time, not just some of the time!  Besides, it clutters up the interface :-)
> 
> 4. Set the zoom and position settings for each image depending on the ZoomMode selected.
> 
> 5. Add a group of three Zoom Mode radio buttons to the Image Settings configuration dialog page: "Autofit each image", "Shared zoom and position" and "Individual (per-image) zoom and position".  I think adding these radio buttons is the most elegant way for users to change this setting, given how non-obvious the current method is (as can be seen by the continual stream of bug reports!).
> 
> 
> Diffs
> -----
> 
>   app/configdialog.cpp a582695 
>   app/imageviewconfigpage.ui f5e1e5a 
>   app/viewmainpage.cpp 7caf099 
>   lib/documentview/documentview.cpp 1ab8bbb 
>   lib/gwenviewconfig.kcfg 92d39f7 
>   lib/zoommode.h PRE-CREATION 
>   lib/zoomwidget.h b181f5a 
>   lib/zoomwidget.cpp da66a56 
> 
> Diff: https://git.reviewboard.kde.org/r/119549/diff/
> 
> 
> Testing
> -------
> 
> Have compiled Gwenview GIT head with this patch on my system; tested successfully with all variants.
> 
> 
> Thanks,
> 
> John Zaitseff
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/gwenview-devel/attachments/20140804/d2e17f3d/attachment.html>