Closed Bug 1621015 Opened 7 months ago Closed 3 months ago

Profiler popup button should be split into a start/stop button, and a down arrow

Categories

(DevTools :: Performance Tools (Profiler/Timeline), enhancement, P2)

enhancement

Tracking

(firefox80 fixed)

RESOLVED FIXED
Firefox 80
Tracking Status
firefox80 --- fixed

People

(Reporter: gregtatum, Assigned: florian)

References

(Depends on 5 open bugs, Blocks 1 open bug)

Details

Attachments

(2 files)

Attached image spec.png

This is a follow-up for the initial rewrite. See the spec.png image.

I spent some time looking into this today, and it looks like it'll be fairly difficult to accomplish. Here's what I've found.

Using dropmarker

There is already a dropmarker in the toolbar, like in the design spec. However, it seems to be an artifact of pre-photon code, and no existing components use it. It's embedded in the toolbarbutton, which means that the command events don't seem to map cleanly to separate buttons.

I tried augmenting the arrow key navigation, which I think is this code, to no avail.

In my initial work, I can't really get it to act like a separate button, and the DOM structure doesn't really match what we're trying to accomplilsh.

Using a custom widget

The widgets support a custom type, and this is what the buttons that are separated out use (e.g. the copy/cut/paste buttons). However, this machinery creates a completely custom widget, without support for the rest of the popup machinery. The if statement in buildWidget is where the custom widgets are implemented. The first branch delegates the button creation to the onBuild command, which returns a node. However, the second "if" branch builds the full popup button as we're currently using it. There is no existing code that is shared here. Plus the second if branch calls out to a lot of internal CustomizableUI which is hard to abstract into something shareable.

(In reply to Greg Tatum [:gregtatum] from comment #1)

I spent some time looking into this today, and it looks like it'll be fairly difficult to accomplish. Here's what I've found.

Using dropmarker

There is already a dropmarker in the toolbar, like in the design spec. However, it seems to be an artifact of pre-photon code, and no existing components use it. It's embedded in the toolbarbutton

Bug 1640493 created a new wantdropmarker attribute that needs to be added for toolbarbuttons to still have a drop marker.

Assignee: nobody → florian
Status: NEW → ASSIGNED

The next version of this patch will depend on the test changes in bug 1625044.

Depends on: 1625044

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:florian, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(florian)
Attachment #9153166 - Attachment description: Bug 1621015 - Profiler popup button should be split into a start/stop button, and a down arrow, r=gregtatum. → Bug 1621015 - Profiler popup button should be split into a start/stop button, and a down arrow, r=julienw.
Pushed by fqueze@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/efa2336315ed
Profiler popup button should be split into a start/stop button, and a down arrow, r=julienw.
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 80
Depends on: 1650830
Depends on: 1650832
Depends on: 1650833
Depends on: 1650835
Depends on: 1650843
Flags: needinfo?(florian)
You need to log in before you can comment on or make changes to this bug.