Bug 391758 - Reorder crop advanced settings
Summary: Reorder crop advanced settings
Status: RESOLVED FIXED
Alias: None
Product: gwenview
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Gwenview Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-12 15:00 UTC by Tsokar
Modified: 2018-03-15 23:02 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Patch for swapping size and position entries in croptool (2.87 KB, patch)
2018-03-14 07:17 UTC, Tsokar
Details
Improved patch (2.56 KB, patch)
2018-03-14 07:23 UTC, Tsokar
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tsokar 2018-03-12 15:00:28 UTC
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 !
Comment 1 null 2018-03-13 00:29:20 UTC
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?
Comment 2 Tsokar 2018-03-14 07:17:26 UTC
Created attachment 111389 [details]
Patch for swapping size and position entries in croptool
Comment 3 Tsokar 2018-03-14 07:18:21 UTC
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
Comment 4 Tsokar 2018-03-14 07:23:22 UTC
Created attachment 111391 [details]
Improved patch

I just realized that I introduced some &amp in the ui file (don't know why).
I just removed them.
Comment 5 null 2018-03-14 09:39:41 UTC
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.

> 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.

> Concerning your question, I use position (and size)
> when I have to crop multiple similar images.
Okay, then we shouldn't remove it probably.
Comment 6 null 2018-03-15 23:02:38 UTC
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