Closed Bug 1459175 Opened 3 years ago Closed 9 months ago

Introduce UI for Options drop down menu in Network panel

Categories

(DevTools :: Netmonitor, enhancement, P2)

enhancement

Tracking

(firefox77 verified)

VERIFIED FIXED
Firefox 77
Tracking Status
firefox77 --- verified

People

(Reporter: Honza, Assigned: davidwalsh, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, good-first-bug)

Attachments

(3 files)

Introduce UI for Options drop down menu that is directly available from within the Network panel toolbar.

Some options like "Persist Logs" and "Disable cache" might stay in the toolbar, but less important options can go into the drop down. Also if there is not enough horizontal space on the toolbar even more important options can move to the drop down.

Honza
@Victoria: which mockup is relevant here?

This is one for the Console panel (and we might want to have unified solution across all panels):
https://mozilla.invisionapp.com/share/2XEEY0RYA#/screens/263400464

And this one is for Network panel:
https://mozilla.invisionapp.com/share/2XEEY0RYA#/screens/263401584

Honza
Flags: needinfo?(victoria)
Priority: -- → P3
Hi Honza,

Based on the conversations and informal testing from before, I think we determined that the gear menu setting with 'compact mode' is just for Console - however, maybe the situation with Network has changed due to the new throttle and HAR dropdowns? I don't know if they will be used seldom enough to warrant being hidden in a menu at all times.

The second mockup you linked to, with an overflow menu that works like the new top-level DevTools tab bar, is probably the better direction for the Network panel, except that we should probably move the filter buttons into a second row before hiding anything into the overflow menu. If this is the behavior, the window needs to be quite small (like less than 450px wide) before any overflow happens. Only very recently could the window even get to be smaller than 450 in docked-to-side mode, so I would say this problem is fairly low priority.
Flags: needinfo?(victoria)
(That's my current understanding - happy to discuss more if you think any overflow/option behavior should work differently :))
Product: Firefox → DevTools

There is also similar bug for the Console panel, see bug 1523868

See the mockup

Honza

Duplicate of this bug: 1617375

We could move these into a dropdown menu:

  • Persist Logs
  • Import HAR…
  • Save All as HAR
  • Copy All as HAR

I would keep visible at all times:

  • Disable Cache
  • Throttling

Since they affect the testing and debugging experience so much, so we should make it hard to miss non-default values.

Mentor: odvarko

Harald, I guess the Network panel options menu should work just like the Console panel options menu, correct?
(could be good first/second bug)

Do we agree on the content as stated in comment #6?

Honza

Flags: needinfo?(hkirschner)

Yes, we should build on the prior art and align as much as possible. Comment 6 summarizes it well!

I would, as a follow up, move the import/export options into its own dropdown as all others are settings (also with consistency in mind across panels, including profiler).

Flags: needinfo?(hkirschner)

Good intermedia bug, building on the prior work in bug 1523868.

Keywords: good-first-bug

I will try to implement this feature. Also this is my second bug so I don't know if you would let me or keep this for the new contributors.

If yes, I would appreciate knowing the files that affect Network panel options and how the console settings (cog icon) is rendered so I can recreate the same for Network panel which in my opinion would clean up the UI a lot and be consistent with the design. I'll try to make it according to the feedback from everyone here.
@Honza: Can you guide me? Thanks

KC, this should be a good second bug. This is the patch that adds the menu for Console, it should provide answers to most of your questions: https://phabricator.services.mozilla.com/D47979

(In reply to KC from comment #10)

@Honza: Can you guide me? Thanks

The patch mentioned by Harald should help a lot.

A few more links:

  1. The place where the button should be rendered
    https://searchfox.org/mozilla-central/rev/2fd8ffcf087bc59a8e5c962965bbb7bf230bcd28/devtools/client/netmonitor/src/components/Toolbar.js#578

Just after the HAR button, you can introduce renderSettingsButton
Note there there are two modes in which the toolbar is rendered (depends on width of the browser/Toolbox window) look for the other occurence of this.renderHarButton()

  1. Existing context menu impl, as an example
    https://searchfox.org/mozilla-central/rev/2fd8ffcf087bc59a8e5c962965bbb7bf230bcd28/devtools/client/netmonitor/src/widgets/RequestListContextMenu.js#43

Look in the same directory for other context menu examples

  1. Drop down button
    render and show Har button show how to impl a drop down button

https://searchfox.org/mozilla-central/rev/2fd8ffcf087bc59a8e5c962965bbb7bf230bcd28/devtools/client/netmonitor/src/components/Toolbar.js#469

https://searchfox.org/mozilla-central/rev/2fd8ffcf087bc59a8e5c962965bbb7bf230bcd28/devtools/client/netmonitor/src/components/Toolbar.js#483

But, now I am seeing that Console is using a nice drop down with a little pointer to the button. It's rendered here:
https://searchfox.org/mozilla-central/rev/2fd8ffcf087bc59a8e5c962965bbb7bf230bcd28/devtools/client/webconsole/components/FilterBar/FilterBar.js#332

We should do it the same way.

Honza

Attached image image.png

Here is how the Console panel is doing it (drop down with a little pointer back to the button). It looks great.

Honza

Assignee: nobody → kartik.c918
Status: NEW → ASSIGNED

Yes I had the same design in mind from console to follow along here for consistency, maybe move Disable Cache and HAR options under the cog drop down? It'll help cleanup the toolbar on smaller widths too.
I'll go through all the resources soon and get started on this.
The commit Harald linked to has helped clear some of my questions. Thanks.

"Disable cache" has a strong impact on the Netmonitor's behavior, which can be not obvious if hidden away, so I'd keep it in the toolbar.

As a first step for this bug I would create a settings menu with only "Persist Logs" in it.

Then we can do follow-up bugs for:

  1. Moving the HAR options to that new menu (likely)
  2. Considering moving "Disable cache" or other actions that new menu (less likely because usage and impact are higher than for HAR)

Agree with Florens

Honza

I'll get started with rendering persist logs in menu.

As a general rule, potential "footguns" (options that severely change browser behavior) need to be in the top-level UI.

KC, are you still interested in fixing this bug or should I unassign it so, somebody else have a chance to finish it?

Honza

Flags: needinfo?(kartik.c918)

I am still interested in fixing this bug. Sorry for the delay, I had an emergency to attend to due to the outbreak which took my last two weeks. I'll begin my work on this from 1st April after being completely free from researching on my GSoC proposal. Sorry and I will provide a patch as soon as possible.

Flags: needinfo?(kartik.c918)

I see, thanks for quick update.

Thanks for the help and take care in these tough times!
Honza

Reassigning this bug to David (per offline chat with :KC)

Honza

Assignee: kartik.c918 → dwalsh
Priority: P3 → P2

Notes:

  • It looks like the button the console uses is shared:
const MenuButton = createFactory(
  require("devtools/client/shared/components/menu/MenuButton")
);

https://searchfox.org/mozilla-central/rev/2cd2d511c0d94a34fb7fa3b746f54170ee759e35/devtools/client/webconsole/components/FilterBar/ConsoleSettings.js#16

  • The tooltip should come for free with that component.
Pushed by dwalsh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/85699ca47925
Introduce UI for Options drop down menu in Network panel r=Honza
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 77

On macOS the dropdown menu used in Console has a border, but not the new one in Network. Needs a follow-up bug?

The separator between "Persist Logs" and "Import HAR File" is a menu item which can receive keyboard focus.

:nchovebbe: How is the Netmonitor's usage of Menu different than webconsole's? I thought I had used the same methodology with this patch but your implemention has a border and appears to live inside a tooltip-xul-wrapper; I can't select any of the menu's UI elements with the browser toolbox. Could you provide any insight?

Flags: needinfo?(nchevobbe)

The MenuButton component takes a toolboxDoc prop that is used to put the menu in the right document.
For the Netmonitor, you pass document (devtools/client/netmonitor/src/components/Toolbar.js#471), which is the netmonitor document, not the toolbox one, which I guess could explain some of the issues.

Also, as Florens said, separator should be implemented with an other element than a MenuItem (usually a hr is what we want)

Flags: needinfo?(nchevobbe)
Regressions: 1631490

Updated the screenshot and description in the Network toolbar article. I called the 'gear' menu an "Action" menu, since the items are all verbs. Only "Persist Logs" is an option.

Looks great to me, thanks Janet!

Honza

QA Whiteboard: [qa-77b-p2]

Verified with 77.0 on Windows 10, macOS 10.15.3, Ubuntu 20.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.