After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 699913 - Selection pattern design updates
Selection pattern design updates
Status: RESOLVED FIXED
Product: gnome-clocks
Classification: Applications
Component: general
3.8.x
Other Linux
: Normal normal
: ---
Assigned To: Clocks maintainer(s)
Clocks maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-05-08 09:20 UTC by Allan Day
Modified: 2013-06-04 21:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch proposal for the ui rework (6.95 KB, patch)
2013-05-24 14:54 UTC, Evgeny Bobkin
needs-work Details | Review
screenshot 1: delete button is not sensible (290.70 KB, image/png)
2013-05-24 15:00 UTC, Evgeny Bobkin
  Details
second screenshot (269.04 KB, image/png)
2013-05-24 15:03 UTC, Evgeny Bobkin
  Details
screenshot 1: delete button is not sensible (256.70 KB, image/png)
2013-05-30 14:25 UTC, Evgeny Bobkin
  Details
second screenshot (236.52 KB, image/png)
2013-05-30 14:26 UTC, Evgeny Bobkin
  Details
screenshot 1: with primariy-toolbar css. delete button is not sensible (215.70 KB, image/png)
2013-06-04 11:22 UTC, Evgeny Bobkin
  Details
screenshot 2: with primariy-toolbar css. delete button is sensitive (218.42 KB, image/png)
2013-06-04 11:26 UTC, Evgeny Bobkin
  Details
screenshot 1: with custom css. delete button is not sensitive (215.02 KB, image/png)
2013-06-04 11:31 UTC, Evgeny Bobkin
  Details
screenshot 2: with custom css. delete button is sensitive (223.49 KB, image/png)
2013-06-04 11:32 UTC, Evgeny Bobkin
  Details
patch proposal for the ui rework (7.76 KB, patch)
2013-06-04 13:02 UTC, Evgeny Bobkin
none Details | Review
patch proposal for the ui rework (8.35 KB, patch)
2013-06-04 14:38 UTC, Evgeny Bobkin
none Details | Review
patch proposal for the ui rework (8.43 KB, patch)
2013-06-04 14:40 UTC, Evgeny Bobkin
none Details | Review

Description Allan Day 2013-05-08 09:20:17 UTC
There's been a round of changes to the selection pattern; these are intended to
overcome a bunch of issues that we've encountered with the existing design.

Details of the updated design can be found here:

https://live.gnome.org/GnomeOS/Design/Whiteboards/SelectionPattern
Comment 1 Paolo Borelli 2013-05-13 19:23:54 UTC
A couple of questions for Allan:

1 - (not specific to clocks) the new pattern makes it less evident, but still has the issue of the action bar potentially covering the some of the selected items... should we scroll the view? should we just live with it? (I am fine with the latter)

2 - Does the bar at the bottom use a standard css class?

3 - For clocks where we just have a single "delete" action, should it go on the left or on the right?


If someone wants to give this task a try it should be fairly easy, the bottom bar can be implement with GtkRevealer.
Comment 2 Evgeny Bobkin 2013-05-24 14:54:38 UTC
Created attachment 245246 [details] [review]
patch proposal for the ui rework
Comment 3 Evgeny Bobkin 2013-05-24 14:59:12 UTC
and here are additional sreenshots for the first draft of the UI selection pattern redesign
Comment 4 Evgeny Bobkin 2013-05-24 15:00:21 UTC
Created attachment 245247 [details]
screenshot 1: delete button is not sensible
Comment 5 Evgeny Bobkin 2013-05-24 15:03:04 UTC
Created attachment 245248 [details]
second screenshot
Comment 6 Paolo Borelli 2013-05-26 17:34:49 UTC
Review of attachment 245246 [details] [review]:

Could you attach screenshots with the default adwaita theme?


Patch looks mostly good, some comments below.

::: src/widgets.vala
@@ +405,3 @@
         scrolled_window.add (icon_view);
 
+        box = new Gtk.Box (Gtk.Orientation.VERTICAL, 0);

I'd prefer to use GtkGrid instead of Box

@@ -465,3 +471,3 @@
     private Gtk.Toolbar create_selection_toolbar () {
         var toolbar = new Gtk.Toolbar ();
-        toolbar.show_arrow = false;
+

I think we should set the "primary toolbar" css class, unless there is a specific class for this bar. Allan?

@@ -467,5 +473,5 @@
-        toolbar.show_arrow = false;
-        toolbar.margin_bottom = 40;
-        toolbar.icon_size = Gtk.IconSize.LARGE_TOOLBAR;
... 2 more ...
+        delete_button.label = _("Delete");
+        delete_button = new Gd.HeaderSimpleButton ();
+        delete_button.get_style_context ().add_class ("suggested-action");
... 2 more ...

I think the Delete button should be at the right since it is the only action in this case.

@@ -515,4 +515,2 @@
     }
 
-    private void fade_in (Gtk.Widget w) {
-        uint timeout_id = w.get_data<uint> ("cloks-fade-out-timeout-id");

The patch should also remove these styles from the css since they are not used anymore
Comment 7 Evgeny Bobkin 2013-05-30 14:24:39 UTC
sure, this was actually the default adwaita theme)) but in dark))
Comment 8 Evgeny Bobkin 2013-05-30 14:25:44 UTC
Created attachment 245645 [details]
screenshot 1: delete button is not sensible
Comment 9 Evgeny Bobkin 2013-05-30 14:26:33 UTC
Created attachment 245646 [details]
second screenshot
Comment 10 Allan Day 2013-05-30 15:39:50 UTC
I really thought I'd replied to this! Sorry to both.

(In reply to comment #1)
> A couple of questions for Allan:
> 
> 1 - (not specific to clocks) the new pattern makes it less evident, but still
> has the issue of the action bar potentially covering the some of the selected
> items... should we scroll the view? should we just live with it? (I am fine
> with the latter)

Yes, scroll the view.

> 2 - Does the bar at the bottom use a standard css class?

Yes, just treat it as a standard primary toolbar.

> 3 - For clocks where we just have a single "delete" action, should it go on the
> left or on the right?

I'd go for right.
Comment 11 Allan Day 2013-05-30 21:36:43 UTC
(In reply to comment #9)
> Created an attachment (id=245646) [details]
> second screenshot

Hey Evgeny! This looks great! Thanks for working on it.

There are a few tweaks that I would recommend:

 * The bottom toolbar is missing a stroke on the top edge. I know that this is something that other applications, like Contacts and Music have had to solve, so I would speak to their developers. You could also ask lapo or cosimoc, since they are our theme experts.

 * As noted above, I would put the delete button on the right. This is mostly because it is a destructive action, so putting it slightly more out of focus feels more appropriate.

 * I wouldn't make the delete button blue, and would instead use the standard style. We use blue for the default action, which shouldn't be coupled to a destructive action like delete.
Comment 12 Evgeny Bobkin 2013-06-04 11:18:39 UTC
(In reply to comment #6)
> Review of attachment 245246 [details] [review]:
> 
> Could you attach screenshots with the default adwaita theme?
> 
> 
> Patch looks mostly good, some comments below.
> 
> ::: src/widgets.vala
> @@ +405,3 @@
>          scrolled_window.add (icon_view);
> 
> +        box = new Gtk.Box (Gtk.Orientation.VERTICAL, 0);
> 
> I'd prefer to use GtkGrid instead of Box
okey
> 
> @@ -465,3 +471,3 @@
>      private Gtk.Toolbar create_selection_toolbar () {
>          var toolbar = new Gtk.Toolbar ();
> -        toolbar.show_arrow = false;
> +
> 
> I think we should set the "primary toolbar" css class, unless there is a
> specific class for this bar. Allan?
I will attach a screenshot of a toolbar with a "primary-toolbar" css set. looks odd to me and the one you have suggested with:

.clocks-selection-bar {
    border-top: 1px;
    border-style: solid;
    border-color: @borders;
}

> 
> @@ -467,5 +473,5 @@
> -        toolbar.show_arrow = false;
> -        toolbar.margin_bottom = 40;
> -        toolbar.icon_size = Gtk.IconSize.LARGE_TOOLBAR;
> ... 2 more ...
> +        delete_button.label = _("Delete");
> +        delete_button = new Gd.HeaderSimpleButton ();
> +        delete_button.get_style_context ().add_class ("suggested-action");
> ... 2 more ...
> 
> I think the Delete button should be at the right since it is the only action in
> this case.
it's from now on on the right hand side
> 
> @@ -515,4 +515,2 @@
>      }
> 
> -    private void fade_in (Gtk.Widget w) {
> -        uint timeout_id = w.get_data<uint> ("cloks-fade-out-timeout-id");
> 
> The patch should also remove these styles from the css since they are not used
> anymore
thanks, indeed this should be removed. I overlooked it.
Comment 13 Evgeny Bobkin 2013-06-04 11:22:46 UTC
Created attachment 245993 [details]
screenshot 1: with primariy-toolbar css. delete button is not sensible
Comment 14 Evgeny Bobkin 2013-06-04 11:26:00 UTC
Created attachment 245994 [details]
screenshot 2: with primariy-toolbar css. delete button is sensitive
Comment 15 Evgeny Bobkin 2013-06-04 11:27:21 UTC
with custom css style:

.clocks-selection-bar {
    border-top: 1px;
    border-style: solid;
    border-color: @borders;
}
Comment 16 Evgeny Bobkin 2013-06-04 11:31:34 UTC
Created attachment 245995 [details]
screenshot 1: with custom css. delete button is not sensitive
Comment 17 Evgeny Bobkin 2013-06-04 11:32:22 UTC
Created attachment 245996 [details]
screenshot 2: with custom css. delete button is sensitive
Comment 18 Evgeny Bobkin 2013-06-04 11:38:39 UTC
(In reply to comment #11)
> (In reply to comment #9)
> > Created an attachment (id=245646) [details] [details]
> > second screenshot
> 
> Hey Evgeny! This looks great! Thanks for working on it.
Hey Allan, it's a pleasure for me. Moreover, I have been accepted and will work within the frame of gsoc on gnome-clocks picture support and geolocation integration in view.
> 
> There are a few tweaks that I would recommend:
> 
>  * The bottom toolbar is missing a stroke on the top edge. I know that this is
> something that other applications, like Contacts and Music have had to solve,
> so I would speak to their developers. You could also ask lapo or cosimoc, since
> they are our theme experts.
Thank you, I have talked with them and Paolo has helped me with that as well.
> 
>  * As noted above, I would put the delete button on the right. This is mostly
> because it is a destructive action, so putting it slightly more out of focus
> feels more appropriate.
done.
> 
>  * I wouldn't make the delete button blue, and would instead use the standard
> style. We use blue for the default action, which shouldn't be coupled to a
> destructive action like delete.
incorporated. So, please take a fresh look)
Comment 19 Evgeny Bobkin 2013-06-04 13:02:45 UTC
Created attachment 246000 [details] [review]
patch proposal for the ui rework
Comment 20 Evgeny Bobkin 2013-06-04 14:38:26 UTC
Created attachment 246009 [details] [review]
patch proposal for the ui rework
Comment 21 Evgeny Bobkin 2013-06-04 14:40:29 UTC
Created attachment 246010 [details] [review]
patch proposal for the ui rework
Comment 22 Allan Day 2013-06-04 14:44:02 UTC
The updated screenshots look perfect! It would be preferable not to use custom css styling here - if you've gone down that route, it would be worth talking to Paolo and the other application developers about getting this fixed in Adwaita.

Thanks again for working on this. :)
Comment 23 Paolo Borelli 2013-06-04 14:55:01 UTC
the custom css is just for the upper border, I think it is ok for now. Once the pattern is standardized we can move it to adwaita and drop our copy


The patch looks good and I will push it asap
Comment 24 Paolo Borelli 2013-06-04 20:53:48 UTC
I have tweaked the css a little bit and pushed. Thanks!
Comment 25 Allan Day 2013-06-04 21:39:16 UTC
Thanks Evgeny! I'm looking forward to trying this.