[prev in list] [next in list] [prev in thread] [next in thread]
List: gwenview-devel
Subject: Re: Review Request 119549: Allow different ways to handle zoom and position between images
From: Aurélien Gâteau <agateau () kde ! org>
Date: 2014-08-07 16:28:30
Message-ID: 20140807162830.20974.80578 () probe ! kde ! org
[Download RAW message or body]
--===============5118399826738306668==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
> On août 4, 2014, 10:44 matin, Aurélien Gâteau wrote:
> > 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
>
> John Zaitseff wrote:
> * Moving the zoom mode block is probably a good idea. Did you want me to redo the patch, or will you? \
> I can post the "git request-pull" here if you like.
> * Renaming "Individual (per image) zoom and position" is also a good idea.
>
> * I'm not so sure about renaming "Shared zoom and position": the point is that the zoom and position \
> are *shared* between different images. I do think it can be worded better, however, so I'm open to \
> suggestions. What about "Keep same zoom and position" (with enum renamed to KeepSame)?
> I can also provide a patch for the documentation (handbook, etc.) to explain all this to the user; I \
> just didn't want to do that before this patch was accepted :-)
> Aurélien Gâteau wrote:
> "Keep same zoom and position" and a KeepSame enum sounds good to me. A documentation update sounds good \
> as well :)
> Would be great if you could update the merge request with these changes as it makes it easy for me to \
> merge them in without having to use git author command line options to attribute the changes to you.
> John Zaitseff wrote:
> I've done all the above, except for the documentation update: that I'll do as a separate patch, as I \
> may well modify more than just my little section. I also plan to add tooltips, etc, to the dialog \
> boxes, as part of the documentation patch.
> John Zaitseff wrote:
> The following changes since commit fde8adf02b8c7ce6b5f2975af3a628daabbe567d:
>
> SVN_SILENT made messages (.desktop file) (2014-07-06 04:29:41 +0000)
>
> are available in the git repository at:
>
> git://git.zap.org.au/data/git/gwenview zoom-modes
>
> for you to fetch changes up to a32c9c127eebe2e90c4af6a9c84a662321d1f80e:
>
> Move the "Zoom mode" block above "Enlarge smaller images" checkbox (2014-08-06 08:56:18 +1000)
>
> ----------------------------------------------------------------
> John Zaitseff (13):
> Add radio buttons to set the default zoom mode. Also renumber all controls on the page to be \
> systematic. Rename Single zoom mode to Shared zoom mode
> Add the ZoomMode config file option
> Remove the superfluous Lock Zoom button in the zoom widget
> Completely remove traces of ZoomLocked
> Use saved zoom and position only for individual zoom mode
> Fix issue where "0% zoom" is shown (bug #335675)
> Add missing ">" to e-mail address
> Remove trailing whitespace
> Replace tabs with spaces in source code
> Rename label "Individual (per-image)" to "Per image zoom and position"
> Rename Shared zoom mode to KeepSame
> Move the "Zoom mode" block above "Enlarge smaller images" checkbox
>
> app/configdialog.cpp | 7 ++
> app/imageviewconfigpage.ui | 226 ++++++++++++++++++++++++++++++--------
> app/kipiinterface.h | 1 -
> app/viewmainpage.cpp | 19 ++--
> cmake/FindLCMS2.cmake | 3 +-
> devdoc/CONTRIBUTING.md | 1 -
> doc/index.docbook | 38 +++----
> lib/archiveutils.h | 1 -
> lib/cms/iccjpeg.c | 2 +-
> lib/documentview/documentview.cpp | 6 +-
> lib/gwenviewconfig.kcfg | 28 +++--
> lib/zoommode.h | 49 +++++++++
> lib/zoomwidget.cpp | 29 -----
> lib/zoomwidget.h | 6 -
> part/gvpart.rc | 1 -
> 15 files changed, 289 insertions(+), 128 deletions(-)
> create mode 100644 lib/zoommode.h
Thanks a lot! I just merged it in master (it's too late for 4.14 unfortunately).
- Aurélien
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119549/#review63738
-----------------------------------------------------------
On août 7, 2014, 6:27 après-midi, John Zaitseff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119549/
> -----------------------------------------------------------
>
> (Updated août 7, 2014, 6:27 après-midi)
>
>
> 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/kipiinterface.h 45fb03b
> app/viewmainpage.cpp 7caf099
> cmake/FindLCMS2.cmake cc17e9f
> devdoc/CONTRIBUTING.md ffdb7f7
> doc/index.docbook d3266b8
> lib/archiveutils.h db79771
> lib/cms/iccjpeg.c 1f6c4b1
> lib/documentview/documentview.cpp 1ab8bbb
> lib/gwenviewconfig.kcfg 92d39f7
> lib/zoommode.h PRE-CREATION
> lib/zoomwidget.h b181f5a
> lib/zoomwidget.cpp da66a56
> part/gvpart.rc 35703a7
>
> 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
>
>
--===============5118399826738306668==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; \
border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119549/">https://git.reviewboard.kde.org/r/119549/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On août 4th, 2014, 10:44 matin CEST, <b>Aurélien Gâteau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Hi, looks great, thanks for the patch!</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I only have minor \
requests for the configuration dialog:<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" />
- 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".<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
- Move the "Zoom mode" block above the "Enlarge smaller images" checkbox to keep all zooming options \
together</p></pre> </blockquote>
<p>On août 5th, 2014, 2:20 matin CEST, <b>John Zaitseff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;"><ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 \
1em;line-height: inherit;white-space: normal;"> <li style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;"> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Moving the zoom mode block is probably a good idea. Did \
you want me to redo the patch, or will you? I can post the "git request-pull" here if you like.</p> \
</li> <li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Renaming "Individual (per image) zoom and position" is also a good idea.</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm \
not so sure about renaming "Shared zoom and position": the point is that the zoom and position are <em \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;">shared</em> between different images. I do think it can be worded better, however, so I'm open \
to suggestions. What about "Keep same zoom and position" (with enum renamed to KeepSame)?</p> </li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I can \
also provide a patch for the documentation (handbook, etc.) to explain all this to the user; I just \
didn't want to do that before this patch was accepted :-)</p></pre> </blockquote>
<p>On août 5th, 2014, 9:33 matin CEST, <b>Aurélien Gâteau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">"Keep same zoom and position" and a KeepSame enum sounds good to me. A \
documentation update sounds good as well :)</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Would be great if you could update the merge request with \
these changes as it makes it easy for me to merge them in without having to use git author command line \
options to attribute the changes to you.</p></pre> </blockquote>
<p>On août 6th, 2014, 1:06 matin CEST, <b>John Zaitseff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I've done all the above, except for the documentation update: that I'll do \
as a separate patch, as I may well modify more than just my little section. I also plan to add tooltips, \
etc, to the dialog boxes, as part of the documentation patch.</p></pre> </blockquote>
<p>On août 6th, 2014, 1:20 matin CEST, <b>John Zaitseff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">The following changes since commit \
fde8adf02b8c7ce6b5f2975af3a628daabbe567d:</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">SVN_SILENT made messages (.desktop file) (2014-07-06 \
04:29:41 +0000)</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">are available in the git repository at:</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">git://git.zap.org.au/data/git/gwenview zoom-modes</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">for you to fetch changes up to \
a32c9c127eebe2e90c4af6a9c84a662321d1f80e:</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Move the "Zoom mode" block above "Enlarge smaller images" \
checkbox (2014-08-06 08:56:18 +1000)</p> <hr style="text-rendering: inherit;margin: 0;padding: \
0;white-space: normal;border: 1px solid #ddd;line-height: inherit;" /> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">John Zaitseff (13):<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /> Add \
radio buttons to set the default zoom mode. Also renumber all controls on the page to be systematic.<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" />
Rename Single zoom mode to Shared zoom mode<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" />
Add the ZoomMode config file option<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" />
Remove the superfluous Lock Zoom button in the zoom widget<br style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: normal;" />
Completely remove traces of ZoomLocked<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" />
Use saved zoom and position only for individual zoom mode<br style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: normal;" />
Fix issue where "0% zoom" is shown (bug #335675)<br style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: normal;" />
Add missing ">" to e-mail address<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" />
Remove trailing whitespace<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" />
Replace tabs with spaces in source code<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> Rename label "Individual (per-image)" to "Per image zoom \
and position"<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" />
Rename Shared zoom mode to KeepSame<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" />
Move the "Zoom mode" block above "Enlarge smaller images" checkbox</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">app/configdialog.cpp | 7 ++<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> app/imageviewconfigpage.ui | 226 \
++++++++++++++++++++++++++++++--------<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> app/kipiinterface.h | 1 -<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /> \
app/viewmainpage.cpp | 19 ++--<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> cmake/FindLCMS2.cmake | 3 +-<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /> \
devdoc/CONTRIBUTING.md | 1 -<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> doc/index.docbook | 38 +++----<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /> \
lib/archiveutils.h | 1 -<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> lib/cms/iccjpeg.c | 2 +-<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /> \
lib/documentview/documentview.cpp | 6 +-<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> lib/gwenviewconfig.kcfg | 28 +++--<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /> \
lib/zoommode.h | 49 +++++++++<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> lib/zoomwidget.cpp | 29 -----<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /> \
lib/zoomwidget.h | 6 -<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> part/gvpart.rc | 1 -<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /> 15 \
files changed, 289 insertions(+), 128 deletions(-)<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> create mode 100644 lib/zoommode.h</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Thanks a lot! I just merged it in master (it's too late for 4.14 \
unfortunately).</p></pre> <br />
<p>- Aurélien</p>
<br />
<p>On août 7th, 2014, 6:27 après-midi CEST, John Zaitseff wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; \
border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Gwenview.</div>
<div>By John Zaitseff.</div>
<p style="color: grey;"><i>Updated août 7, 2014, 6:27 après-midi</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=337262">337262</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
gwenview
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid \
#b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">(Taken from bug #337262 )</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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...</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I have come to realise \
that there are THREE main ways people like to have the zoom and position settings applied to successive \
images:</p> <ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: \
inherit;white-space: normal;"> <li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;"> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">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.</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p> </li>
</ol>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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).</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">I am submitting a patch that does the following:</p> <ol style="padding: 0;text-rendering: \
inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;"> <li style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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).</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Remove \
the now-obsolete ShowLockZoomButton and LockZoom config options.</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 :-)</p> \
</li> <li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Set \
the zoom and position settings for each image depending on the ZoomMode selected.</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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!).</p> </li>
</ol></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Have compiled Gwenview GIT head with this \
patch on my system; tested successfully with all variants.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>app/configdialog.cpp <span style="color: grey">(a582695)</span></li>
<li>app/imageviewconfigpage.ui <span style="color: grey">(f5e1e5a)</span></li>
<li>app/kipiinterface.h <span style="color: grey">(45fb03b)</span></li>
<li>app/viewmainpage.cpp <span style="color: grey">(7caf099)</span></li>
<li>cmake/FindLCMS2.cmake <span style="color: grey">(cc17e9f)</span></li>
<li>devdoc/CONTRIBUTING.md <span style="color: grey">(ffdb7f7)</span></li>
<li>doc/index.docbook <span style="color: grey">(d3266b8)</span></li>
<li>lib/archiveutils.h <span style="color: grey">(db79771)</span></li>
<li>lib/cms/iccjpeg.c <span style="color: grey">(1f6c4b1)</span></li>
<li>lib/documentview/documentview.cpp <span style="color: grey">(1ab8bbb)</span></li>
<li>lib/gwenviewconfig.kcfg <span style="color: grey">(92d39f7)</span></li>
<li>lib/zoommode.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/zoomwidget.h <span style="color: grey">(b181f5a)</span></li>
<li>lib/zoomwidget.cpp <span style="color: grey">(da66a56)</span></li>
<li>part/gvpart.rc <span style="color: grey">(35703a7)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119549/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============5118399826738306668==--
_______________________________________________
Gwenview-devel mailing list
Gwenview-devel@kde.org
https://mail.kde.org/mailman/listinfo/gwenview-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic