818447 - [toolbox] Make the "Web Developer" appmenu entry a combo button (Linux & Windows).

Status: RESOLVED FIXED

(DevTools :: General, defect, P2)

Description

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Like the "new tab" button.

Comment 1

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Original Github issue:
https://github.com/joewalker/devtools-window/issues/186

Comment 2

[Girish Sharma [:Optimizer]]

Do we want to remove the "Toggle Toolbox" entry after the "Web Developer" menu in app menu becomes a combo button ?

As I think clicking on Web Developer would itself open the Toolbox.

Comment 3

[Paul Rouget [:paul]]

I think we want. Let's see how it feels.

Comment 4

[Girish Sharma [:Optimizer]]

Attachment: Combo button

Made the appmenu Web Developer entry a combo box.

But now we have no information of whether a toolbox is opened for that tab through app menu.

Comment 5

[Girish Sharma [:Optimizer]]

Attachment: Combo button

Made Web Developer entry a combo box in App menu.

But now we have lost the way to know whether toolbox is opened for that tab via app menu.

Comment 6

[Paul Rouget [:paul]]

Comment on attachment 692463 [details] [diff] [review]
Combo button

>-      <menu id="appmenu_webDeveloper"
>-            label="&appMenuWebDeveloper.label;">
>+      <splitmenu id="appmenu_webDeveloper"
>+                 observes="devtoolsMenuBroadcaster_WebDeveloper"
>+                 label="&appMenuWebDeveloper.label;">
>         <menupopup id="appmenu_webDeveloper_popup">
>-          <menuitem id="appmenu_devToolbox"
>-                    observes="devtoolsMenuBroadcaster_DevToolbox"/>
>-          <menuseparator id="appmenu_devtools_separator"/>
>           <menuitem id="appmenu_devToolbar"
>                     observes="devtoolsMenuBroadcaster_DevToolbar"/>

We want to keep this menuitem: the combo button is not obvious and can be missed.

>+#ifndef XP_MACOSX
>+    <broadcaster id="devtoolsMenuBroadcaster_WebDeveloper"
>+                 />
>+#endif

I would not create a new <broadcaster>, and instead of observers=…, I'd use command="Tools:DevToolbox".

Comment 7

[Girish Sharma [:Optimizer]]

Attachment: patch v2.0

You just made the patch so easy..

Comment 8

[Paul Rouget [:paul]]

(In reply to Girish Sharma [:Optimizer] from comment #7)
> You just made the patch so easy..

Yeah, I'm like that :D

Comment 9

[Paul Rouget [:paul]]

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

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/c55a8f678488