1271708 - Inspector context menu: remove items "expand all", "collapse" and "Duplicate node"

Status: RESOLVED INVALID

(DevTools :: Inspector, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

Follow-up to Bug 1211613 comment 25 :

(In reply to Patrick Brosset <:pbro> from comment #25)
> Comment on attachment 8740538 [details] [diff] [review]
> Reordering the items in context menu and moving copy and paste items in
> submenus.
> 
> Review of attachment 8740538 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This is a great start, makes the menu a lot easier to navigate. Thanks.
> I think we should land this and then perhaps do another iteration in another
> bug to discuss items such as "duplicate node" or "collapse" and "expand"
> which aren't in comment 10 but are still in the menu.

Comment 1

[Julian Descottes [:jdescottes]]

Helen, Patrick: Now that Bug 1211613 has landed, how do you feel about those remaining context menu items:
- Duplicate Node
- Expand all
- Collapse

(personally, I think "Expand all"/"Collapse" are too redundant with existing UI elements to be worth having here. "Duplicate Node" can not be done anywhere else though ?)

Comment 2

[Helen V. Holmes (:helenvholmes) (:✨)]

Whoops, sorry about clearing my needinfo.

I agree about the Expand All/Collapse items. Brian and I when we were working on the list here: https://bugzilla.mozilla.org/show_bug.cgi?id=1211613#c10 we got rid of duplicate node because it didn't seem like it was needed/used a lot.

Sole: in bug 1211613, we reorganized and removed some of the options in the right-click context menu in the Inspector to make it more useable. In that bug, we removed Duplicate Node, Expand all, and Collapse. Could you alert us if any of those changes are taken negatively?

Comment 3

[Brian Grinstead [:bgrins]]

(In reply to Helen V. Holmes (:helenvholmes) (:✨)(pls ni?) from comment #2)
> Sole: in bug 1211613, we reorganized and removed some of the options in the
> right-click context menu in the Inspector to make it more useable. In that
> bug, we removed Duplicate Node, Expand all, and Collapse. Could you alert us
> if any of those changes are taken negatively?

To be clear, we discussed/agreed on removing these things but didn't end up doing it in that bug as per https://bugzilla.mozilla.org/show_bug.cgi?id=1211613#c23.  We could use this bug for that though

Comment 4

[Julian Descottes [:jdescottes]]

That's why I logged this follow-up, sorry if it was unclear.

Patrick's comment (Bug 1211613 comment 25) mentioned "discussing" the remaining items so I assumed we were unsure about the next steps.

Updating summary and marking as good first bug.

Comment 5

[Brian Grinstead [:bgrins]]

(In reply to Julian Descottes [:jdescottes] from comment #4)
> That's why I logged this follow-up, sorry if it was unclear.
> 
> Patrick's comment (Bug 1211613 comment 25) mentioned "discussing" the
> remaining items so I assumed we were unsure about the next steps.
> 
> Updating summary and marking as good first bug.

Can we wait until Bug 1266478 lands?  It's a big patch in flight and I'd have to rebase around any change to the context menu.

Comment 6

[Helen V. Holmes (:helenvholmes) (:✨)]

(In reply to Julian Descottes [:jdescottes] from comment #4)
> That's why I logged this follow-up, sorry if it was unclear.
> 
> Patrick's comment (Bug 1211613 comment 25) mentioned "discussing" the
> remaining items so I assumed we were unsure about the next steps.
> 
> Updating summary and marking as good first bug.

Oh, whoops—my fault!

Let's use this bug to handling removing them and its discussion. We'll wait until Bug 1266478 lands.

I'll flag Sole for an ni? once the changes in this bug land.

Comment 7

[Patrick Brosset <:pbro>]

(In reply to Julian Descottes [:jdescottes] from comment #1)
> Helen, Patrick: Now that Bug 1211613 has landed, how do you feel about those
> remaining context menu items:
> - Duplicate Node
We don't have the data to know whether this is used much. My gut feeling is that this is probably rarely used. It is very handy when prototyping in the browser, adding DOM elements, adding styles, to create a quick layout prototype or something. I know I've used it a few times in this context. But most people consider the inspector as an "inspection" tool (hence the name) rather than an "authoring" tool. I wouldn't be surprised if people mostly used the "edit as html" option rather than this to create more similar elements.
So I'm opposed to removing it, or maybe keeping it as a niche feature only behind a shortcut key? Just like the S to scroll, H to hide, DEL to remove?
> - Expand all
We added this originally only behind alt+click on the expander (which still exists) for Chrome parity.
Again, we don't have the data to know if it's used.
> - Collapse
Ok to remove this, the expanded icon is more than obvious for this. No need for an extra menu item.

Comment 8

[Helen V. Holmes (:helenvholmes) (:✨)]

(In reply to Patrick Brosset <:pbro> from comment #7)
> > - Duplicate Node
> So I'm opposed to removing it, or maybe keeping it as a niche feature only
> behind a shortcut key? Just like the S to scroll, H to hide, DEL to remove?
Did you mean not opposed to removing it, Patrick? I'd be pretty happy to keep all of these behind key commands.

> > - Expand all
> We added this originally only behind alt+click on the expander (which still
> exists) for Chrome parity.
> Again, we don't have the data to know if it's used.
Chrome seems to have removed it, looking at release (or perhaps I'm looking in the wrong place?).

Comment 9

[Patrick Brosset <:pbro>]

(In reply to Helen V. Holmes (:helenvholmes) (:✨)(pls ni?) from comment #8)
> (In reply to Patrick Brosset <:pbro> from comment #7)
> > > - Duplicate Node
> > So I'm opposed to removing it, or maybe keeping it as a niche feature only
> > behind a shortcut key? Just like the S to scroll, H to hide, DEL to remove?
> Did you mean not opposed to removing it, Patrick? I'd be pretty happy to
> keep all of these behind key commands.
Yes, sorry, that's what I meant.
> > > - Expand all
> > We added this originally only behind alt+click on the expander (which still
> > exists) for Chrome parity.
> > Again, we don't have the data to know if it's used.
> Chrome seems to have removed it, looking at release (or perhaps I'm looking
> in the wrong place?).
They still have alt+click, I don't know if they ever had an item in the context menu, but they don't have one now.
We first added alt+click for parity in FF31 (bug 997622) and the corresponding item in the context menu came later in FF44 (bug 1157789). So I'm happy if we keep alt+click and want to get rid of the menu item.

Comment 10

[Patrick Brosset <:pbro>]

Inspector bug triage (filter on CLIMBING SHOES).

Comment 11

[Tim Nguyen :ntim]

(In reply to Patrick Brosset <:pbro> from comment #7)
> (In reply to Julian Descottes [:jdescottes] from comment #1)
> > Helen, Patrick: Now that Bug 1211613 has landed, how do you feel about those
> > remaining context menu items:
> > - Duplicate Node
> We don't have the data to know whether this is used much. My gut feeling is
> that this is probably rarely used. It is very handy when prototyping in the
> browser, adding DOM elements, adding styles, to create a quick layout
> prototype or something. I know I've used it a few times in this context. But
> most people consider the inspector as an "inspection" tool (hence the name)
> rather than an "authoring" tool. I wouldn't be surprised if people mostly
> used the "edit as html" option rather than this to create more similar
> elements.
Yeah, I introduced the menu item because it's useful for authoring.

> So I'm not opposed to removing it, or maybe keeping it as a niche feature only
> behind a shortcut key? Just like the S to scroll, H to hide, DEL to remove?
+1, I'm fine with having it under Cmd/Ctrl+D (which seems natural, since most tools use this shortcut).

Comment 12

[John Hawkinson]

(In reply to Patrick Brosset <:pbro> from comment #9)
> We first added alt+click for parity in FF31 (bug 997622) and the
> corresponding item in the context menu came later in FF44 (bug 1157789). So
> I'm happy if we keep alt+click and want to get rid of the menu item.

Coming late to the party, as I migrate from Firebug to Firefox's devtools.

In Firebug, Expand All was bound to *. This was very handy for keyboard navigation, esp. as mouse navigation of hierarchical tree elements can require repetitive and excessive dexterity whereas keyboard arrows are very handy and quick.

Removal of the context menu will mean there is no way to have a discoverable keyboard shortcut bound to Expand All, because the way to discover shortcuts is to view them in the context menu.
I think that's a problem, and on that basis would oppose removal of Expand All from the context menu.

(Right now, today, there is no keyboard shortcut assigned to Expand All. I find that frustrating, but I don't want to submit an enhancement request for that if Expand All is going to be removed from the context menu and thus obviate the utility of the shortcut. Please let me know if I'm wrong on this and should go ahead and file.)

(A related wart is that Expand All ought to be paired with a complimentary Collapse All, and that functionality is missing. Both in the Alt-click mouse UI as well as the context menu. So if I Expand All a 7-level deep hierarchy, such as when I am quickly trying to find something that is nested deeply, and then want to collapse all 7 levels, there is no way to do so. Expand All becomes one-way functionality. This is at odds to normal expand/collapse all functionality in other tools, including Firebug. It would be my intention to file that as an enhancement request, but I'll wait a bit to hear any feedback or context in this thread.)

Comment 13

[Julian Descottes [:jdescottes]]

(In reply to John Hawkinson from comment #12)
> (In reply to Patrick Brosset <:pbro> from comment #9)
> > We first added alt+click for parity in FF31 (bug 997622) and the
> > corresponding item in the context menu came later in FF44 (bug 1157789). So
> > I'm happy if we keep alt+click and want to get rid of the menu item.
> 
> Coming late to the party, as I migrate from Firebug to Firefox's devtools.
> 
> In Firebug, Expand All was bound to *. This was very handy for keyboard
> navigation, esp. as mouse navigation of hierarchical tree elements can
> require repetitive and excessive dexterity whereas keyboard arrows are very
> handy and quick.
> 
> Removal of the context menu will mean there is no way to have a discoverable
> keyboard shortcut bound to Expand All, because the way to discover shortcuts
> is to view them in the context menu.
> I think that's a problem, and on that basis would oppose removal of Expand
> All from the context menu.
> 
> (Right now, today, there is no keyboard shortcut assigned to Expand All. I
> find that frustrating, but I don't want to submit an enhancement request for
> that if Expand All is going to be removed from the context menu and thus
> obviate the utility of the shortcut. Please let me know if I'm wrong on this
> and should go ahead and file.)
> 
> (A related wart is that Expand All ought to be paired with a complimentary
> Collapse All, and that functionality is missing. Both in the Alt-click mouse
> UI as well as the context menu. So if I Expand All a 7-level deep hierarchy,
> such as when I am quickly trying to find something that is nested deeply,
> and then want to collapse all 7 levels, there is no way to do so. Expand All
> becomes one-way functionality. This is at odds to normal expand/collapse all
> functionality in other tools, including Firebug. It would be my intention to
> file that as an enhancement request, but I'll wait a bit to hear any
> feedback or context in this thread.)

"Collapse all" for alt+click just landed in Bug 1401889. I wanted to file a follow-up bug to propose replacing "collapse" by "collapse all" in the menu. 

I agree with what you said, both collapse all and expand all can be hard to discover if you've never used the feature in a similar tree. We should keep them in the menu. Adding a keyboard shortcut could be a next step. 

So summarizing the items discussed in the bug , I would say:
- Expand all: keep it
- Collapse: replace it with Collapse all
- Duplicate node: remove 

gl: are you ok with ^ ? If yes we should file two dedicated bugs (should both be good-first-bug)

We can also think about adding a keyboard shortcut and surface it in the context menu. John, feel free to log a bug for this.

Comment 14

[Gabriel Luong [:gl] (ΦωΦ)]

I would prefer to keep Duplicate node since I found that to be very useful. So, perhaps let's not take any action on the Duplicate node for now. 

I am ok with replacing Collapse with Collapse all.

Adding keyboard shortcut in the context menu is great, but I would like to be also mindful of the shortkey tooltip that debugger.html is introducing and hopefully we can also leverage that in the future.

Comment 15

[Julian Descottes [:jdescottes]]

Works for me, logged Bug 1405246 for Collapse->Collapse all

Closing this one. If we want to discuss "Duplicate node" specifically it should go into a new bug.