796148 - Implement a 'gear menu'

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Victor Porof [:vporof][:vp]]

We'll soon have two checkboxes in the debugger frontend UI: 'Pause on exceptions' and 'Hide non-enumerable properties'. Probably more in the near future. A gear-like menu holding all these options (like the Inspector used to have) is mandatory.

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Old, bitrotted patch

Here is an old patch from the pause-on-exceptions bug that displayed it as an option in a gear menu. It may be a good starting point.

Comment 2

[Victor Porof [:vporof][:vp]]

(In reply to Panos Astithas [:past] from comment #1)
> Created attachment 666918 [details] [diff] [review]
> Old, bitrotted patch
> 
> Here is an old patch from the pause-on-exceptions bug that displayed it as
> an option in a gear menu. It may be a good starting point.

Thanks!

Comment 3

[Victor Porof [:vporof][:vp]]

I'm having a hard time deciding where to put this button. On the left? Right? Inspector had it on the left. I think it should be on the right (think of Australis menus).

Comment 4

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Victor Porof [:vp] from comment #3)
> I'm having a hard time deciding where to put this button. On the left?
> Right? Inspector had it on the left. I think it should be on the right
> (think of Australis menus).

I think it would make more sense on the right, especially if we move the close and full-screen buttons over there, as Brian suggested. But let's hear it from the horse's mouth :-)

Comment 5

[Victor Porof [:vporof][:vp]]

Attachment: v1

There.

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 674591 [details] [diff] [review]
v1

Review of attachment 674591 [details] [diff] [review]:
-----------------------------------------------------------------

Nice! Perhaps the close button is special, but why don't you move the 'fullscreen' icon to the right as well, so that the gear doesn't feel lonely?

::: browser/locales/en-US/chrome/browser/devtools/debugger.dtd
@@ +36,5 @@
>  <!ENTITY debuggerUI.closeButton.tooltip "Close">
>  
> +<!-- LOCALIZATION NOTE (debuggerUI.prefsButton.tooltip): This is the tooltip for
> +  -  the button that opens up an options context menu for the debugger UI. -->
> +<!ENTITY debuggerUI.prefsButton.tooltip "Preferences">

Didn't we use 'Options' for the inspector gear menu? Does anyone remember? Does it matter?

@@ +47,5 @@
>  <!-- LOCALIZATION NOTE (debuggerUI.showNonEnums): This is the label for the
>    -  checkbox that toggles visibility of hidden (non-enumerable) variables and
>    -  properties in stack views. -->
>  <!ENTITY debuggerUI.showNonEnums        "Show hidden properties">
> +<!ENTITY debuggerUI.showNonEnums.key    "N">

Wouldn't 'H' make more sense, being the first letter of hidden?

::: browser/themes/winstripe/jar.mn
@@ +379,5 @@
>          skin/classic/aero/browser/devtools/splitview.css             (devtools/splitview.css)
>          skin/classic/aero/browser/devtools/styleeditor.css           (devtools/styleeditor.css)
>          skin/classic/aero/browser/devtools/debugger.css              (devtools/debugger.css)
>          skin/classic/aero/browser/devtools/magnifying-glass.png      (devtools/magnifying-glass.png)
> +        skin/classic/aera/browser/devtools/option-icon.png           (devtools/option-icon.png)

Typo: aero

Comment 7

[Victor Porof [:vporof][:vp]]

(In reply to Panos Astithas [:past] from comment #6)
> Comment on attachment 674591 [details] [diff] [review]
> v1
> 
> Review of attachment 674591 [details] [diff] [review]:
> -----------------------------------------------------------------
> 

Thanks!

> Nice! Perhaps the close button is special, but why don't you move the
> 'fullscreen' icon to the right as well, so that the gear doesn't feel lonely?
> 

Ok.

> ::: browser/locales/en-US/chrome/browser/devtools/debugger.dtd
> @@ +36,5 @@
> >  <!ENTITY debuggerUI.closeButton.tooltip "Close">
> >  
> > +<!-- LOCALIZATION NOTE (debuggerUI.prefsButton.tooltip): This is the tooltip for
> > +  -  the button that opens up an options context menu for the debugger UI. -->
> > +<!ENTITY debuggerUI.prefsButton.tooltip "Preferences">
> 
> Didn't we use 'Options' for the inspector gear menu? Does anyone remember?
> Does it matter?
> 

Yeah, checked bug 724509, we used "Inspector Options". I'll change.

> @@ +47,5 @@
> >  <!-- LOCALIZATION NOTE (debuggerUI.showNonEnums): This is the label for the
> >    -  checkbox that toggles visibility of hidden (non-enumerable) variables and
> >    -  properties in stack views. -->
> >  <!ENTITY debuggerUI.showNonEnums        "Show hidden properties">
> > +<!ENTITY debuggerUI.showNonEnums.key    "N">
> 
> Wouldn't 'H' make more sense, being the first letter of hidden?
> 

How about when we'll get "Hide foobarbaz"? I can change it to H, but I'm curious about what you think.

Comment 8

[Victor Porof [:vporof][:vp]]

(In reply to Victor Porof [:vp] from comment #7)
> 
> How about when we'll get "Hide foobarbaz"? I can change it to H, but I'm
> curious about what you think.

Or maybe P for 'properties'?

Comment 9

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Victor Porof [:vp] from comment #8)
> (In reply to Victor Porof [:vp] from comment #7)
> > 
> > How about when we'll get "Hide foobarbaz"? I can change it to H, but I'm
> > curious about what you think.
> 
> Or maybe P for 'properties'?

Using 'F' for foobarbaz would be OK I'd think. But 'P' is OK, too.

Comment 10

[Victor Porof [:vporof][:vp]]

Green on try: https://tbpl.mozilla.org/?tree=Try&rev=b21791666650

Comment 11

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/integration/fx-team/rev/0eecbc50194c

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0eecbc50194c

Comment 13

[rexyrexy2]

please change it back to a gear and stop mimicking chrome... D:

Comment 14

[Victor Porof [:vporof][:vp]]

(In reply to rexyrexy2 from comment #13)
> please change it back to a gear and stop mimicking chrome... D:

It is a gear.