GNOME Bugzilla – Bug 699910
Selection pattern design updates
Last modified: 2013-08-08 09:47:36 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
*** Bug 687649 has been marked as a duplicate of this bug. ***
Created attachment 250837 [details] [review] selections: Implement the new selection toolbar
Created attachment 250838 [details] [review] mainToolbar: Use a "Cancel" button instead of "Done"
Created attachment 250848 [details] [review] selections: Exit the selection mode when an action has been performed
I don't know what the behaviour should be when performing an action opens a modal dialog. Should we exit from the selection mode immediately when the dialog is opened? Or should we only exit when the dialog has been destroyed?
Review of attachment 250837 [details] [review]: Thanks, looks good to me.
Review of attachment 250837 [details] [review]: Thanks, looks good to me. ::: src/selections.js @@ +742,1 @@ + let toolbar = new Gtk.HeaderBar(); Actually, thinking more about this, can't you just use a GtkBox instead?
Review of attachment 250838 [details] [review]: Sure
Review of attachment 250848 [details] [review]: Looks good other than the comment below. ::: src/selections.js @@ +796,3 @@ + this._setItemVisibility(); + this.widget.set_reveal_child(true); I feel this should be part of the first patch maybe?
(In reply to comment #5) > I don't know what the behaviour should be when performing an action opens a > modal dialog. Should we exit from the selection mode immediately when the > dialog is opened? Or should we only exit when the dialog has been destroyed? I think it should exit the mode when the dialog is destroyed; it's useful to see the selection in the background and changing it while at the same time opening a dialog might feel a little too noisy. Allan, Jon, what do you think?
(In reply to comment #7) > Review of attachment 250837 [details] [review]: > > Thanks, looks good to me. > > ::: src/selections.js > @@ +742,1 @@ > + let toolbar = new Gtk.HeaderBar(); > > Actually, thinking more about this, can't you just use a GtkBox instead? I found Gtk.HeaderBar to be more convenient because: - You get the behaviour of the pack_start and pack_end methods for free. Otherwise, to have a bunch of buttons on the left and few on the right, you have to do some magic yourself. This used to be the case before. - You also get the spacing/padding between the buttons/bar for free. Also, gnome-weather uses a Gtk.HeaderBar. So ... :-)
(In reply to comment #10) > (In reply to comment #5) > > I don't know what the behaviour should be when performing an action opens a > > modal dialog. Should we exit from the selection mode immediately when the > > dialog is opened? Or should we only exit when the dialog has been destroyed? > > I think it should exit the mode when the dialog is destroyed; it's useful to > see the selection in the background and changing it while at the same time > opening a dialog might feel a little too noisy. > Allan, Jon, what do you think? After talking to Jon, we decided to quit the selection mode if the response from the dialog was positive. In case of a negative response (say "Cancel") we should stay in the mode.
Created attachment 250876 [details] [review] selections: Implement the new selection toolbar
Created attachment 250877 [details] [review] selections: Exit the selection mode when an action has been performed
(In reply to comment #9) > Review of attachment 250848 [details] [review]: > > Looks good other than the comment below. > > ::: src/selections.js > @@ +796,3 @@ > > + this._setItemVisibility(); > + this.widget.set_reveal_child(true); > > I feel this should be part of the first patch maybe? Done.
Review of attachment 250876 [details] [review]: Looks good
Review of attachment 250877 [details] [review]: Sure
Created attachment 251006 [details] [review] selections: Tighten the conditions for the actions in selection mode
Created attachment 251008 [details] [review] selections: Use labels instead of tooltips for Open
Review of attachment 251006 [details] [review]: I might already have fixed this with a slightly different patch in git master yesterday - can you see if this still applies?
Review of attachment 251008 [details] [review]: Looks good
(In reply to comment #20) > Review of attachment 251006 [details] [review]: > > I might already have fixed this with a slightly different patch in git master > yesterday - can you see if this still applies? Yes, your patch already fixes it and it takes a more elegant approach than mine. This patch is no longer needed.