Zoom Icons could be improved
Submitted by an unknown user
Link to original bug (#717234)
Description
---- Reported by shotwell-maint@gnome.bugs 2010-12-29 08:07:00 -0800 ----
Original Redmine bug id: 3037
Original URL: http://redmine.yorba.org/issues/3037
Searchable id: yorba-bug-3037
Original author: Andrew -
Original description:
Whilst I like how the current zoom icons are more visually intuitive than the standard magnifying glass, I feel they could be improved.
-
The people in the icons are a bit blurred
-
Because they are images, the colours chosen can conflict heavily with the theme chosen
---- Additional Comments From shotwell-maint@gnome.bugs 2013-05-16 14:44:00 -0700 ----
History
Comment 1
Updated by Andrew - almost 3 years ago
Attached is a a patch whilst solves these issues.
Instead of using images, which are rendered to Gdk.Pixbufs, the patch makes use of the Gtk.EventBoxs already used by shotwell for registering Gdk.Events. The code in the patch draws the icon onto the EventBox and uses colours form the user's GTK theme – this way the colours never conflict with the theme.
Attached is the patch which makes it all happen, that can be committed to trunk, and also a screenshot that shows the difference the patch makes, with three different themes (before is on the left, after on the right).
Comment 2
Updated by Andrew - almost 3 years ago
One thing I noticed – I didn't see a major difference in the zoom slider code being used by the photopage and the mediapage, is there a reason why a common class can't be created, that these two could share?
Comment 3
Updated by Andrew - almost 3 years ago
- Subject changed from Zoom Icons could he improved to Zoom Icons could be improved
Comment 4
Updated by Lucas Beeler almost 3 years ago
is there a reason why a common class can't be created, that these two could share?
This is a consequence of the fact that the application-level objects that the two sliders control are quite different: the MediaPage slider has fixed endpoint values and controls the parameters of a CheckerboardLayout. The PhotoPage slider has variable endpoint values (per-photo) and controls a fairly complex thingamajig called a ZoomBuffer. That said, I think you've got a point and there might be some utility in just packing the slider endpoint icons and the slider control itself into an HBox and making it a custom widget class whose signals are similar to those of the Slider control.
Comment 5
Updated by Adam Dingle almost 3 years ago
and471,
thanks for the patch. We looked at this and discussed yesterday. We like the fact that your new zoom icons have colors which don't conflict in any theme. In the default Ubuntu Ambiance theme your icons have a satisfying gray color. Unfortunately, in Clearlooks (the default GNOME theme) with your patch the zoom icons are a solid black color which looks too bold and distracting.
If you could rework the patch so that the zoom icons look nicer (i.e. grayer) in Clearlooks, then we would be interested in committing this. Of course, we don't want to have a workaround in our code that looks for any theme specifically. Perhaps you could retrieve a different color from the theme to use for drawing the icons, or perhaps you could detect when the color is completely black or white and make it grayer in that case.
Comment 6
Updated by Andrew - almost 3 years ago
Okay here is a reworked patch, it checks if the color is too strong, and if so, reduces it's lightness value.
Comment 7
Updated by Adam Dingle almost 3 years ago
- Target version set to 0.9
- Priority set to High
Thanks for this latest patch. The colors now look nice, and this is a definite improvement on dark themes such as High Contrast Inverse. I think I slightly prefer the smaller icon sizes in Shotwell today rather than the larger icons in your patch, but the crisper icons are possibly an improvement, and the dark theme color improvement is probably what's most important anyway.
Jim, can you review the code? When we're all around at Yorba we should look at this change visually together.
Comment 8
Updated by Adam Dingle over 2 years ago
-
Target version deleted (
<strike>
_0.9_</strike>
)
A bit late to look at this now for 0.9. We should review for 0.10.
Comment 9
Updated by Jim Nelson about 1 year ago
- Description updated (diff)
- Assignee set to Lucas Beeler
- Target version set to 0.14.0
Comment 10
Updated by Lucas Beeler about 1 year ago
- Status changed from Review to Open
-
Assignee deleted (
<strike>
_Lucas Beeler_</strike>
)
@Andrew:
Are you still around? We're actually very interested in taking this patch but would like to see it updated to work with the current git master, including Gtk+ 3.x support, etc. If you're still subscribed, are you willing to update your patch such that it is diff'd against the current Shotwell git master? Thanks!
Comment 11
Updated by Jim Nelson 11 months ago
- Category set to ux
Comment 12
Updated by Jim Nelson 10 months ago
- Target version changed from 0.14.0 to 0.15.0
Comment 13
Updated by Jim Nelson 8 months ago
- Target version changed from 0.15.0 to 0.16.0
Comment 14
Updated by Jim Nelson 6 months ago
-
Target version deleted (
<strike>
_0.16.0_</strike>
)
--- Bug imported by chaz@yorba.org 2013-11-25 21:49 UTC ---
This bug was previously known as bug 3037 at http://redmine.yorba.org/show_bug.cgi?id=3037 Imported an attachment (id=261928) Imported an attachment (id=261929) Imported an attachment (id=261930)
Unknown version " in product shotwell. Setting version to "!unspecified". Unknown milestone "unknown in product shotwell. Setting to default milestone for this product, "---". Setting qa contact to the default for this product. This bug either had no qa contact or an invalid one. Resolution set on an open status. Dropping resolution
Resolution: RESOLVED OBSOLETE