Closed Bug 1548703 Opened 7 months ago Closed 5 months ago

Markup View: Contextual Menu Rejiggering

Categories

(DevTools :: Inspector, enhancement, P3)

enhancement

Tracking

(firefox69 fixed)

RESOLVED FIXED
Firefox 69
Tracking Status
firefox69 --- fixed

People

(Reporter: victoria, Assigned: gl)

References

Details

Attachments

(2 files, 2 obsolete files)

Attached image image.png (obsolete) —

Here is a bug about making the Contextual menu more compact and convenient. This work sprouted out of the DOM breakpoints project.

Current version attached. Ongoing work on mockup will be at this link: https://mozilla.invisionapp.com/share/25RTZ229FKU

Feedback welcome!

Attached image image.png (obsolete) —
Attachment #9062376 - Attachment is obsolete: true

Ideally, telemetry could be added to each menu item so we can continue to improve this based on usage data.

Looks great overall.

Copy-paste

  • Do we remove the ability to copy the element.innerHTML code? (That would be okay for me, I rarely use it.)
  • I think the default Paste action (Cmd+V) should be "Paste>After". That way, when you select a node and hit Cmd+C then Cmd+V, it duplicated the node in-place, as siblings, a bit like copy-pasting a file in a file explorer. Hitting Ctrl+V ten times would give you 10 copies as siblings.

For instance if I select the wrapper element in:

<div id="wrapper">
  <header>...</header>
  <main>...</main>
</div>

On pasting twice I would rather have this:

<div id="wrapper">
  <header>...</header>
  <main>...</main>
</div>
<div id="wrapper">
  <header>...</header>
  <main>...</main>
</div>
<div id="wrapper">
  <header>...</header>
  <main>...</main>
</div>

than this:

<div id="wrapper">
  <header>...</header>
  <main>...</main>
  <div id="wrapper">
    <header>...</header>
    <main>...</main>
  </div>
  <div id="wrapper">
    <header>...</header>
    <main>...</main>
  </div>
</div>

Ordering following the DOM order?

I think some of the submenus could be ordered in a way that follows a "walking the DOM from this node" order.
Not 100% sure that's better, just an idea.

For instance:

  • Add: Attribute, Div Child, Div After, Duplicate
  • Break on...: Node Removal, Attribute Modification, Subtree Removal

Show Accessibility Info

Can we make it look enabled at all times, at least if the Accessibility panel is in the list of enabled panels? Then on click open the Accessibility panel and store a reference to the requested node, so that if the user chooses to enable Accessibility inspecting we can show the info for the requested node right away? (Same pattern as a log-in page that redirects you to the originally requested URL.)

Screenshot Node

One cool power user feature we have with the :screenshot command is the ability to pick a different resolution. You can :screenshot --dpr 4 --selector ".some-selector" to get a high-res bitmap of a specific element. I'm seeing tips for doing that in Chrome DevTools (and sometimes in Firefox DevTools) on Twitter regularly, and for both it's a bit arcane how to do it. (Chrome does it via its command palette, but there are several steps involved.)

So I wonder if we could offer a submenu like:

Screenshot  >
   This Element
   Visible Page
   Full Page
   -----------------
   Custom Command...

And clicking "Custom Command..." could open the split console with a pre-filled :screenshot --selector "<unique selector for this element>" --dpr <current resolution> maybe?

(This reminds me that it could be great to have a "default screenshot resolution" preference in DevTools. Lots of developers are on 1x screens -- for at least some of their screens -- and might still want to share 2x or higher res screenshots with clients, project managers, etc.)

Type: defect → enhancement

Do we remove the ability to copy the element.innerHTML code? (That would be okay for me, I rarely use it.)

Yeah, to me it seems weirdly indirect compared to just copy outerHTML, plus Chrome doesn't have it. (They seem to redundantly have both Copy Element and Copy outerHTML.)

I think the default Paste action (Cmd+V) should be "Paste>After"

You're right. I got confused when playing around in my file system, but Paste>After seems more correct indeed.

Ordering following the DOM order?

I generally tend towards ordering in usage frequency, keeping in mind logical groupings. Of course, I can only make guesses here. But Attribute seems less useful to me than the others because it's a more easy to do without the menu, so I think I do want it at the end. Maybe Add Div Child is more common than Div After? Though it's also possible with the + button.

Show Accessibility Info
Can we make it look enabled at all times, at least if the Accessibility panel is in the list of enabled panels? Then on click open the Accessibility panel and store a reference to the requested node, so that if the user chooses to enable Accessibility inspecting we can show the info for the requested node right away

This sounds really good! I think I'll make it a separate bug because it's more complicated than just rejiggering.

Screenshot Node

This idea also sounds good for a separate bug.

I updated the mockup and am feeling pretty good about it at this point :)

Assignee: nobody → gl
Status: NEW → ASSIGNED
Priority: -- → P3
Attached image image.png

Latest version of mockup is in this link: https://mozilla.invisionapp.com/share/25RTZ229FKU#/screens

(marking attachments as obsolete)

Attachment #9062377 - Attachment is obsolete: true

This patch rearranges the second and third group of context menu items
in the markup context menu.

@rcaliman, the patch only implements the second and third group of the mockup since we are still missing some items in the first group such as Paste. I want to land the second and third group rearrangement in the meantime to unblock the work for adding the break on DOM mutation context menu in Bug 1550030.

Blocks: 1550030
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e85689f036c3
Rearrange the markup context menu items. r=rcaliman
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69
You need to log in before you can comment on or make changes to this bug.