GNOME Bugzilla – Bug 762942
tool-crop: Update the crop presets
Last modified: 2016-03-05 10:47:16 UTC
Update the crop palette to match the list as per the mockup: https://dl.dropboxusercontent.com/u/5031519/photos/crop-3.20.png
Created attachment 322833 [details] [review] tool-crop: Update crop presets
Created attachment 322836 [details] [review] tool-crop: Update crop presets
Review of attachment 322836 [details] [review]: Thanks for the patch, Umang. ::: src/photos-tool-crop.c @@ +137,3 @@ + { PHOTOS_TOOL_CROP_ASPECT_RATIO_BASIS, N_("3x2 / 6x4"), 3, 2 }, + { PHOTOS_TOOL_CROP_ASPECT_RATIO_BASIS, N_("16x10 (1920x1080)"), 16, 10 }, + { PHOTOS_TOOL_CROP_ASPECT_RATIO_BASIS, N_("16x9 (1280x800)"), 16, 9 } We should use the proper Unicode character (ie. '×') instead of 'x'. See https://wiki.gnome.org/Design/OS/Typography
Created attachment 322864 [details] [review] tool-crop: Update crop presets I fixed the Unicode issue.
Review of attachment 322864 [details] [review]: Something for you to fix: ::: src/photos-tool-crop.c @@ +132,3 @@ + { PHOTOS_TOOL_CROP_ASPECT_RATIO_ORIGINAL, N_("Original"), 0, 0 }, + { PHOTOS_TOOL_CROP_ASPECT_RATIO_BASIS, N_("1×1 (Square)"), 1, 1 }, + { PHOTOS_TOOL_CROP_ASPECT_RATIO_BASIS, N_("10×8 / 5×4"), 5, 4 }, All the presets should be in landscape orientation. That means the first number (ie. the height) should be less than the second (ie. the width).
Created attachment 322866 [details] [review] tool-crop: Update crop presets
Can you please attach screenshots with and without your patch?
Created attachment 322939 [details] Crop presets (without patch)
Created attachment 322940 [details] Updated crop presets
(In reply to Debarshi Ray from comment #7) > Can you please attach screenshots with and without your patch? Done.
Review of attachment 322866 [details] [review]: Thanks, Umang. This is perfect. Since this affects user-facing strings, we would need permission [1] from the translation team before pushing. [1] https://wiki.gnome.org/ThreePointNineteen
Just a little remark on the patch: 1920x1080 is actually 16x9 and 1280x800 is 16x10.
(In reply to Abderrahim Kitouni from comment #12) > Just a little remark on the patch: 1920x1080 is actually 16x9 and 1280x800 > is 16x10. Of course, you are right. Embarassing.
Created attachment 323007 [details] [review] tool-crop: Update crop presets
Created attachment 323010 [details] Updated crop presets
Comment on attachment 323007 [details] [review] tool-crop: Update crop presets Permission granted. Pushed to master.
Thanks for the quick fix, Umang.
I'll add a short question that I had when translating gnome-photos. These are ratios, so is there a reason that they are written as for example 16×10 rather than 16∶10? With 16×10 my first guess would be that something is 16×10 cm or 16×10 inches.
(In reply to Anders Jonsson from comment #18) > I'll add a short question that I had when translating gnome-photos. These > are ratios, so is there a reason that they are written as for example 16×10 > rather than 16∶10? I will let Allan answer that. If you look at the screenshots then you will notice that it used to be the way you are suggesting here. > With 16×10 my first guess would be that something is 16×10 cm or 16×10 > inches. Would a comment be helpful?