Discussion:
[gwenview] [Bug 391758] New: Reorder crop advanced settings
(too old to reply)
Tsokar
2018-03-12 15:00:28 UTC
Permalink
Raw Message
https://bugs.kde.org/show_bug.cgi?id=391758

Bug ID: 391758
Summary: Reorder crop advanced settings
Product: gwenview
Version: unspecified
Platform: Other
OS: Linux
Status: UNCONFIRMED
Severity: normal
Priority: NOR
Component: general
Assignee: gwenview-bugs-***@kde.org
Reporter: ***@crans.org
Target Milestone: ---

Hi !
crop advances settings has a problem of usability (I think).
The settings are entered in the following order :
1/ aspect ratio
2/ position
3/ size

Once these settings have been set for an image, and if we want to reuse them
for the next image, we have to re-enter them in a different order :
1/ aspect radio (if needed)
2/ size
3/ position

If one wants to modify position first (i.e. entering the settings in the order
they appear), it is not possible as the crop window still as its full size (and
thus it cannot be translated)

Thank you very much for your work !
--
You are receiving this mail because:
You are watching all bug changes.
Nate Graham
2018-03-12 18:40:50 UTC
Permalink
Raw Message
https://bugs.kde.org/show_bug.cgi?id=391758

Nate Graham <***@kde.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@kde.org
--
You are receiving this mail because:
You are watching all bug changes.
Henrik Fehlauer
2018-03-13 00:29:20 UTC
Permalink
Raw Message
https://bugs.kde.org/show_bug.cgi?id=391758

Henrik Fehlauer <***@lab12.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@lab12.net

--- Comment #1 from Henrik Fehlauer <***@lab12.net> ---
Thanks for your comments. Just before you submitted Bug 391757, we were just
starting to discuss improvements to the crop tool, see
https://phabricator.kde.org/D11202.

As for swapping "size" and "position", that sounds good. Do you want to submit
a patch for this? Let us know in case you are interested, we can help you of
course.

Also it would be interesting to know how important "position" is, i.e. are you
using this setting at all?
--
You are receiving this mail because:
You are watching all bug changes.
Tsokar
2018-03-14 07:17:26 UTC
Permalink
Raw Message
https://bugs.kde.org/show_bug.cgi?id=391758

--- Comment #2 from Tsokar <***@crans.org> ---
Created attachment 111389
--> https://bugs.kde.org/attachment.cgi?id=111389&action=edit
Patch for swapping size and position entries in croptool
--
You are receiving this mail because:
You are watching all bug changes.
Tsokar
2018-03-14 07:18:21 UTC
Permalink
Raw Message
https://bugs.kde.org/show_bug.cgi?id=391758

--- Comment #3 from Tsokar <***@crans.org> ---
Thank you for your answer !
Please find attached a patch to swap position and size. I hope that everything
is OK, because that's only my second contribution to a kde project.

Concerning your question, I use position (and size) when I have to crop
multiple similar images. I use them to ensure that the images are all exactly
cropped the same way.

Best regards,
Gregory
--
You are receiving this mail because:
You are watching all bug changes.
Tsokar
2018-03-14 07:23:22 UTC
Permalink
Raw Message
https://bugs.kde.org/show_bug.cgi?id=391758

--- Comment #4 from Tsokar <***@crans.org> ---
Created attachment 111391
--> https://bugs.kde.org/attachment.cgi?id=111391&action=edit
Improved patch

I just realized that I introduced some &amp in the ui file (don't know why).
I just removed them.
--
You are receiving this mail because:
You are watching all bug changes.
Henrik Fehlauer
2018-03-14 09:39:41 UTC
Permalink
Raw Message
https://bugs.kde.org/show_bug.cgi?id=391758

--- Comment #5 from Henrik Fehlauer <***@lab12.net> ---
Nice, thanks for the patch. Your first contribution was ten years ago, wow!

I tested your patch, it works fine and looks good to me. You even changed the
tabstops.

However, other members of Gwenview should also get a chance to comment, for
which we use Phabricator nowadays. Would you be able to resubmit your patch
there? (Sorry for not mentioning this earlier.) See
https://community.kde.org/Infrastructure/Phabricator#Basic_Tasks.
Post by Tsokar
I just realized that I introduced some &amp in the ui file (don't know why).
That's a known problem (in your case affecting Qt Designer), see
https://github.com/GoldenCheetah/GoldenCheetah/wiki/Working-around-KDE-shortcut-injection.
Post by Tsokar
Concerning your question, I use position (and size)
when I have to crop multiple similar images.
Okay, then we shouldn't remove it probably.
--
You are receiving this mail because:
You are watching all bug changes.
Henrik Fehlauer
2018-03-15 23:02:38 UTC
Permalink
Raw Message
https://bugs.kde.org/show_bug.cgi?id=391758

Henrik Fehlauer <***@lab12.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Latest Commit| |https://commits.kde.org/gwe
| |nview/8d61a5b957908d7e3313c
| |924471cedb82f188a2f
Status|UNCONFIRMED |RESOLVED

--- Comment #6 from Henrik Fehlauer <***@lab12.net> ---
Git commit 8d61a5b957908d7e3313c924471cedb82f188a2f by Henrik Fehlauer, on
behalf of Gregory Legrain.
Committed on 15/03/2018 at 23:02.
Pushed by rkflx into branch 'master'.

Swap position and size in crop tool

Summary:
When one wants to re-use crop settings in gwenview (i.e. position and size),
they have to be entered in the following order :
1/ Size
2/ Position

(it is not possible to do it the other way, as position is locked until the
size of the crop area is smaller than the size of the image).

However, the settings are displayed in the reverse order, which is not
convenient (un-necessary mouse actions).

Reviewers: #gwenview, rkflx

Reviewed By: #gwenview, rkflx

Subscribers: muhlenpfordt, rkflx, ngraham

Tags: #gwenview

Differential Revision: https://phabricator.kde.org/D11329

M +10 -10 lib/crop/cropwidget.ui

https://commits.kde.org/gwenview/8d61a5b957908d7e3313c924471cedb82f188a2f
--
You are receiving this mail because:
You are watching all bug changes.
Loading...