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 710295 - delete action too close to pause
delete action too close to pause
Status: RESOLVED FIXED
Product: gnome-boxes
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: 3.22
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-10-16 16:10 UTC by Matthias Clasen
Modified: 2016-03-31 13:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
selectionbar: Move delete button to the right (2.16 KB, patch)
2015-03-04 15:17 UTC, Adrien Plazas
needs-work Details | Review
selectionbar: Set remove as destructive (829 bytes, patch)
2015-03-04 15:17 UTC, Adrien Plazas
none Details | Review
selectionbar: Remove properties button (2.73 KB, patch)
2015-03-04 15:17 UTC, Adrien Plazas
none Details | Review
selectionbar: Remove properties button (2.71 KB, patch)
2015-03-04 16:53 UTC, Adrien Plazas
accepted-commit_now Details | Review
selectionbar: Move delete button to the end (791 bytes, patch)
2015-03-04 16:54 UTC, Adrien Plazas
committed Details | Review
selectionbar: Set delete as destructive (804 bytes, patch)
2015-03-04 16:54 UTC, Adrien Plazas
committed Details | Review

Description Matthias Clasen 2013-10-16 16:10:22 UTC
(from Montreal summit session)

It is uncomfortable to have a destructive action like 'Delete' sit so close to the harmless 'Pause' action.

It would also be nice to use a symbolic pause icon instead of the label.
Comment 1 Zeeshan Ali 2013-11-01 15:43:20 UTC
(In reply to comment #0)
> (from Montreal summit session)
> 
> It is uncomfortable to have a destructive action like 'Delete' sit so close to
> the harmless 'Pause' action.

Sure, makes sense.

> It would also be nice to use a symbolic pause icon instead of the label.

Wha? We recently moved all these buttons from icons to labels based on the new GNOME3 trend.
Comment 2 Zeeshan Ali 2014-02-21 01:32:54 UTC
(In reply to comment #1)
> (In reply to comment #0)
> > (from Montreal summit session)
> > 
> > It is uncomfortable to have a destructive action like 'Delete' sit so close to
> > the harmless 'Pause' action.
> 
> Sure, makes sense.

But where shall I put it? In the middle? Would look really weird. I would have the same issue if I put it on the right side next to properties button.
Comment 3 Lasse Schuirmann 2014-05-28 18:57:57 UTC
Just some raw ideas:

* Clocks does have the delete button on the right. Can we think of another way to access the properties so that the delete button can go there? In theory this bottom-bar is for multi-VM-actions. Showing the properties of one isn't this kind of action.

* I think I have seen it colored red in some gnome application. Don't remember where.
Comment 4 Zeeshan Ali 2015-02-23 18:46:36 UTC
(In reply to Lasse Schuirmann from comment #3)
> Just some raw ideas:
> 
> * Clocks does have the delete button on the right. Can we think of another
> way to access the properties so that the delete button can go there? In
> theory this bottom-bar is for multi-VM-actions. Showing the properties of
> one isn't this kind of action.

Hmm.. yeah, we now have context menu to go to properties too so maybe we could simply replace properties button with that.

Jimmac?

> * I think I have seen it colored red in some gnome application. Don't
> remember where.

Another good idea.
Comment 5 Adrien Plazas 2015-03-04 15:17:51 UTC
Created attachment 298543 [details] [review]
selectionbar: Move delete button to the right

This moves the delete button to the right of the selectionbar.
Comment 6 Adrien Plazas 2015-03-04 15:17:55 UTC
Created attachment 298544 [details] [review]
selectionbar: Set remove as destructive

This adds the destructive-action style class to remove button.
Comment 7 Adrien Plazas 2015-03-04 15:17:59 UTC
Created attachment 298545 [details] [review]
selectionbar: Remove properties button

This removes the properties button.
Comment 8 Lasse Schuirmann 2015-03-04 15:40:45 UTC
Review of attachment 298543 [details] [review]:

Commit message isn't really good because you don't move the button but you switch those two. IMO it would make more sense to first remove the properties button, then move the delete one to the right and then mark as destructive.

Looks good otherwise.
Comment 9 Lasse Schuirmann 2015-03-04 15:40:46 UTC
Review of attachment 298543 [details] [review]:

Commit message isn't really good because you don't move the button but you switch those two. IMO it would make more sense to first remove the properties button, then move the delete one to the right and then mark as destructive.

Looks good otherwise.
Comment 10 Lasse Schuirmann 2015-03-04 15:41:38 UTC
Review of attachment 298544 [details] [review]:

::: src/selectionbar.vala
@@ +19,2 @@
     construct {
+        remove_btn.get_style_context ().add_class ("destructive-action");

can't this be done in the XML definition? Seperating UI and code as much as possible.
Comment 11 Lasse Schuirmann 2015-03-04 15:43:06 UTC
Rest looks good for me, I'll do a more thorough review in the next patch iteration.
Comment 12 Adrien Plazas 2015-03-04 16:53:54 UTC
Created attachment 298552 [details] [review]
selectionbar: Remove properties button

This removes the properties button from the selectionbar.
Comment 13 Adrien Plazas 2015-03-04 16:54:03 UTC
Created attachment 298553 [details] [review]
selectionbar: Move delete button to the end

This moves the delete button to the end of the selectionbar.
Comment 14 Adrien Plazas 2015-03-04 16:54:08 UTC
Created attachment 298554 [details] [review]
selectionbar: Set delete as destructive

This adds the destructive-action style class to delete button.
Comment 15 Lasse Schuirmann 2015-03-04 17:33:06 UTC
Review of attachment 298552 [details] [review]:

ack
Comment 16 Lasse Schuirmann 2015-03-04 17:33:22 UTC
Review of attachment 298553 [details] [review]:

ack
Comment 17 Lasse Schuirmann 2015-03-04 17:33:44 UTC
Review of attachment 298554 [details] [review]:

ack, thats better :)
Comment 18 Lasse Schuirmann 2015-03-04 17:34:41 UTC
pushing trivial commits without zeeshan's review.
Comment 19 Lasse Schuirmann 2015-03-04 17:38:01 UTC
Sorry, it's UI freeze, so I've reverted them at an instant. Zeeshan, sorry for this mess :(
Comment 20 Zeeshan Ali 2015-03-04 19:12:05 UTC
(In reply to Lasse Schuirmann from comment #19)
> Sorry, it's UI freeze, so I've reverted them at an instant. Zeeshan, sorry
> for this mess :(

No worries. Feel free to create a gnome-3-17 branch already and push these there.
Comment 21 Lasse Schuirmann 2015-03-04 19:37:58 UTC
(In reply to Zeeshan Ali (Khattak) from comment #20)
> (In reply to Lasse Schuirmann from comment #19)
> > Sorry, it's UI freeze, so I've reverted them at an instant. Zeeshan, sorry
> > for this mess :(
> 
> No worries. Feel free to create a gnome-3-17 branch already and push these
> there.

Done.
Comment 22 Zeeshan Ali 2015-05-22 16:34:18 UTC
Comment on attachment 298552 [details] [review]
selectionbar: Remove properties button

We totally forgot about these patches and I only remembered this bug after I already re-did this patch from scratch. No biggie, its a small patch. I'm obsoleting this one cause mine provides better commit log. :P
Comment 23 Zeeshan Ali 2015-05-22 16:36:50 UTC
Attachment 298553 [details] pushed as 627c43f - selectionbar: Move delete button to the end
Attachment 298554 [details] pushed as 840e8f2 - selectionbar: Set delete as destructive

Also pushed:

commit: 534d3f010115b1bdd81b63df0a289c896508fe58

    selectionbar: Drop Properties button
    
    Not only this is available in both context menu and hamburger (display)
    menu, this is the only single item action in the selection mode and
    selection mode is meant for multiple item actions.