Merge lp:~victored/pantheon-photos/lp-1275117 into lp:~pantheon-photos/pantheon-photos/trunk

Proposed by Victor Martinez
Status: Merged
Approved by: Danielle Foré
Approved revision: 2790
Merged at revision: 2783
Proposed branch: lp:~victored/pantheon-photos/lp-1275117
Merge into: lp:~pantheon-photos/pantheon-photos/trunk
Diff against target: 211 lines (+94/-5)
3 files modified
src/CheckerboardLayout.vala (+48/-3)
src/Page.vala (+41/-1)
src/Resources.vala (+5/-1)
To merge this branch: bzr merge lp:~victored/pantheon-photos/lp-1275117
Reviewer Review Type Date Requested Status
Danielle Foré ux Approve
elementary Apps team code Pending
Review via email: mp+264215@code.launchpad.net

Commit message

Show selector icon when hovering an item. lp: 1275117

Description of the change

Show selector icon when hovering an item. lp: 1275117

To post a comment you must log in.
Revision history for this message
Danielle Foré (danrabbit) wrote :

Is there a way to move the icon further into the top left of the item? Other than that, seems to look and work as expected :) Thanks Victor!

review: Approve (ux)
Revision history for this message
Danielle Foré (danrabbit) wrote :

Oh derp. Icon size should be 24px to match. Dunno how I missed that ;p

Revision history for this message
Victor Martinez (victored) wrote :

Cool. Thanks for the review. I'll apply those fixes :)

Revision history for this message
Victor Martinez (victored) wrote :

By the way, how much closer to the top-left should I move the icon? Currently it's 6px away from the border top left border (both horizontally and vertically)

Revision history for this message
Danielle Foré (danrabbit) wrote :

To be honest, I think I would just put it immediately in the corner of the allocation without any offsets (including the frame and border).

Revision history for this message
Victor Martinez (victored) wrote :

got it. I'll try to move it there.

2787. By Victor Martinez

Reduce selector icon size to 24 px and remove offsets. Now the icon is positioned at the beginning of the allocation.

Revision history for this message
Victor Martinez (victored) wrote :

I've removed every offset and reduced the icon size to 24px. Now the icon is at the corner of the allocation.

Keep in mind the icon is not exactly centered in the corner of the visible border. I hope that's ok :P

2788. By Victor Martinez

Fix minor issue with selections. Previously unselecting an item other than the last one selected resulted in the entire selection being cleared out.

2789. By Victor Martinez

cleanup

Revision history for this message
Danielle Foré (danrabbit) wrote :

Hm, the icon is blurry for HiDPI. It looks like you're using a pixbuf directly here. Any way to use GIcon or something? http://valadoc.org/#!api=gtk+-3.0/Gtk.Image.gicon

Revision history for this message
Victor Martinez (victored) wrote :

Yea I'm rendering the pixbuf directly since the entire widget is rendered with Cairo. And HiDPI never crossed my mind :P

I'll look into it. Can you provide a screenshot of how it looks currently?

2790. By Victor Martinez

Render icon with HiDPI support

Revision history for this message
Victor Martinez (victored) wrote :

HiDPI rendering added :)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/CheckerboardLayout.vala'
2--- src/CheckerboardLayout.vala 2014-08-21 00:59:32 +0000
3+++ src/CheckerboardLayout.vala 2015-07-12 03:17:26 +0000
4@@ -115,6 +115,7 @@
5 public const int TRINKET_PADDING = 1;
6
7 public const int BRIGHTEN_SHIFT = 0x18;
8+ public const int SELECTION_ICON_SIZE = 24;
9
10 public Dimensions requisition = Dimensions ();
11 public Gdk.Rectangle allocation = Gdk.Rectangle ();
12@@ -435,6 +436,15 @@
13 return origin;
14 }
15
16+ public Gdk.Rectangle get_selection_button_area () {
17+ Gdk.Rectangle selection_button_area = Gdk.Rectangle ();
18+ selection_button_area.x = allocation.x;
19+ selection_button_area.y = allocation.y;
20+ selection_button_area.width = SELECTION_ICON_SIZE;
21+ selection_button_area.height = SELECTION_ICON_SIZE;
22+ return selection_button_area;
23+ }
24+
25 protected virtual void paint_shadow (Cairo.Context ctx, Dimensions dimensions, Gdk.Point origin,
26 int radius, float initial_alpha) {
27 double rgb_all = 0.0;
28@@ -529,7 +539,7 @@
29 }
30
31 public void paint (Cairo.Context ctx, Gdk.RGBA bg_color, Gdk.RGBA selected_color,
32- Gdk.RGBA text_color, Gdk.RGBA? border_color) {
33+ Gdk.RGBA text_color, Gdk.RGBA? border_color, int scale_factor) {
34 // calc the top-left point of the pixbuf
35 Gdk.Point pixbuf_origin = Gdk.Point ();
36 pixbuf_origin.x = allocation.x + FRAME_WIDTH + BORDER_WIDTH;
37@@ -549,8 +559,12 @@
38 ctx.restore ();
39 }
40
41+ string? selection_icon = null;
42+
43 // draw selection border
44 if (is_selected ()) {
45+ selection_icon = Resources.ICON_SELECTION_CHECKED;
46+
47 // border thickness depends on the size of the thumbnail
48 ctx.save ();
49 paint_border (ctx, pixbuf_dim, pixbuf_origin,
50@@ -574,6 +588,37 @@
51 ctx.restore ();
52 }
53
54+ // decide which icon to use for the selection button
55+ if (brightened != null && pixbuf != null) {
56+ if (is_selected ())
57+ selection_icon = Resources.ICON_SELECTION_REMOVE;
58+ else
59+ selection_icon = Resources.ICON_SELECTION_ADD;
60+ }
61+
62+ Gdk.Pixbuf? selection_icon_pix = null;
63+ if (selection_icon != null) {
64+ try {
65+ selection_icon_pix = Gtk.IconTheme.get_default ().load_icon_for_scale (selection_icon,
66+ SELECTION_ICON_SIZE, scale_factor, Gtk.IconLookupFlags.GENERIC_FALLBACK);
67+ } catch (Error err) {
68+ warning ("Could not load %s: %s", selection_icon, err.message);
69+ }
70+ }
71+
72+ // draw selector icon
73+ if (selection_icon_pix != null) {
74+ // calc the top-left point of the selection button
75+ Gdk.Rectangle selection_icon_area = get_selection_button_area ();
76+ ctx.save ();
77+ ctx.scale (1.0 / scale_factor, 1.0 / scale_factor);
78+ int icon_x = selection_icon_area.x * scale_factor;
79+ int icon_y = selection_icon_area.y * scale_factor;
80+ Gdk.cairo_set_source_pixbuf (ctx, selection_icon_pix, icon_x, icon_y);
81+ ctx.paint ();
82+ ctx.restore ();
83+ }
84+
85 ctx.set_source_rgba (text_color.red, text_color.green, text_color.blue, text_color.alpha);
86
87 // title and subtitles are LABEL_PADDING below bottom of pixbuf
88@@ -1781,7 +1826,7 @@
89 // have all items in the exposed area paint themselves
90 foreach (CheckerboardItem item in intersection (visible_page)) {
91 item.paint (ctx, bg_color, item.is_selected () ? selected_color : unselected_color,
92- unselected_color, border_color);
93+ unselected_color, border_color, scale_factor);
94 }
95 } else {
96 // draw the message in the center of the window
97@@ -1869,4 +1914,4 @@
98
99 return base.focus_out_event (event);
100 }
101-}
102\ No newline at end of file
103+}
104
105=== modified file 'src/Page.vala'
106--- src/Page.vala 2015-02-22 09:34:49 +0000
107+++ src/Page.vala 2015-07-12 03:17:26 +0000
108@@ -1270,6 +1270,7 @@
109 protected CheckerboardItem cursor = null;
110 private CheckerboardItem highlighted = null;
111 private bool autoscroll_scheduled = false;
112+ private bool selection_button_clicked = false;
113 private CheckerboardItem activated_item = null;
114 private Gee.ArrayList<CheckerboardItem> previously_selected = null;
115
116@@ -1576,6 +1577,8 @@
117 }
118
119 protected override bool on_left_click (Gdk.EventButton event) {
120+ selection_button_clicked = false;
121+
122 // only interested in single-click and double-clicks for now
123 if ((event.type != Gdk.EventType.BUTTON_PRESS) && (event.type != Gdk.EventType.2BUTTON_PRESS))
124 return false;
125@@ -1586,6 +1589,7 @@
126 // use clicks for multiple selection and activation only; single selects are handled by
127 // button release, to allow for multiple items to be selected then dragged
128 CheckerboardItem item = get_item_at_pixel (event.x, event.y);
129+
130 if (item != null) {
131 switch (state) {
132 case Gdk.ModifierType.CONTROL_MASK:
133@@ -1625,6 +1629,37 @@
134 break;
135
136 default:
137+ // check if user clicked a blank area of the item or the selection button
138+ Gdk.Rectangle button_area = item.get_selection_button_area ();
139+
140+ // The user does not have to click exactly over button area for the click to
141+ // be registered as valid. We virtually extend the clickable area around button.
142+ const int x_error_margin = 12;
143+ const int y_error_margin = 12;
144+
145+ if (event.x >= button_area.x - x_error_margin
146+ && event.x <= button_area.x + button_area.width + x_error_margin
147+ && event.y >= button_area.y - y_error_margin
148+ && event.y <= button_area.y + button_area.height + y_error_margin)
149+ {
150+ debug ("Selection button clicked");
151+
152+ // make sure we handle this kind of selection properly on button-release
153+ selection_button_clicked = true;
154+
155+ // when selection button is clicked, multiple selections are possible ...
156+ // chosen item is toggled
157+ Marker marker = get_view ().mark (item);
158+ get_view ().toggle_marked (marker);
159+
160+ if (item.is_selected ()) {
161+ anchor = item;
162+ cursor = item;
163+ }
164+
165+ break;
166+ }
167+
168 if (event.type == Gdk.EventType.2BUTTON_PRESS) {
169 activated_item = item;
170 } else {
171@@ -1695,6 +1730,11 @@
172 return true;
173 }
174
175+ if (selection_button_clicked) {
176+ selection_button_clicked = false;
177+ return true;
178+ }
179+
180 if (cursor != item) {
181 // user released mouse button after moving it off the initial item, or moved from dead
182 // space onto one. either way, unselect everything
183@@ -2730,4 +2770,4 @@
184 warning (err.message);
185 }
186 }
187-}
188\ No newline at end of file
189+}
190
191=== modified file 'src/Resources.vala'
192--- src/Resources.vala 2015-07-08 01:22:24 +0000
193+++ src/Resources.vala 2015-07-12 03:17:26 +0000
194@@ -108,6 +108,10 @@
195 public const string ICON_ZOOM_OUT = "zoom-out-symbolic";
196 public const int ICON_ZOOM_SCALE = 16;
197
198+public const string ICON_SELECTION_ADD = "selection-add";
199+public const string ICON_SELECTION_CHECKED = "selection-checked";
200+public const string ICON_SELECTION_REMOVE = "selection-remove";
201+
202 public const string ICON_CAMERAS = "camera-photo";
203 public const string ICON_EVENTS = "office-calendar";
204 public const string ICON_ONE_EVENT = "office-calendar";
205@@ -1196,4 +1200,4 @@
206
207 public const string ONIMAGE_FONT_COLOR = "#000000";
208 public const string ONIMAGE_FONT_BACKGROUND = "rgba(255,255,255,0.5)";
209-}
210\ No newline at end of file
211+}

Subscribers

People subscribed via source and target branches