firefox-devtools / ux

Firefox DevTools UX Community
Mozilla Public License 2.0
103 stars 21 forks source link

DOM Mutation Breakpoints: Milestone 1 #63

Closed violasong closed 4 years ago

violasong commented 5 years ago

PRD: Milestone 1

Bug 1547692

Engineers: @loganfsmyth, @darkwing & @gabrielluong

darkwing commented 4 years ago

Re-upping this. One requirement from the DOM Mutation Breakpoints PRD was:

Shared panel between Debugger & Inspector

1. Use CSS Rule as preview
2. Highlight Element on hover
3. Target-icon to navigate to Element
4. Checkbox to enable/disable breakpoint
5. Right-click Remove Single and All Breakpoints

These all make sense for the debugger but where do we plan on showing this panel in the inspector?

dom-bps-where
digitarald commented 4 years ago

Just re-iterating, the same panel should be shown in Inspector and Debugger.

  1. Use CSS Rule as preview

See review of Chrome's panel. DOM BP entries are shown using the type of breakpoints and the related element. This specific bullet point is about how the selected element is presented – using the CSS selector, similar to the Grid list in the Inspector.

digitarald commented 4 years ago

First shot at 2 different formats to display the BPs: https://www.figma.com/file/E0q9c7EToCsz8eyq944CEa/DOM-Mutation-Breakpoints?node-id=0%3A1

fvsch commented 4 years ago

So the question for the DOM Breakpoints pane seems to be if we should let users disable all DOM breakpoints for a given node (like Chrome does), or let them disable each type of breakpoint for each node?

Schematically:

[x] ul.examples-detail-list
    Node Removal

or:

ul.examples-detail-list
    [x] Node Removal

I think we could offer both, like we do for Event Listener Breakpoints?

[x] ul.examples-detail-list
    [x] Node Removal

[-] nav#super-nav
    [ ] Node Removal
    [x] Node Insertion

[ ] div.some-other.stuff
    [ ] No Idea What I'm Doing

Or is that useless for users and ultimately just slowing them down?

digitarald commented 4 years ago

Schematically

Now I wonder why I spend so much time in Figma when you solved this problem in ASCII mockups so much better 👍

I think we could offer both, like we do for Event Listener Breakpoints?

Making this a tree would probably be overkill; I even have some doubts that the hierarchical layout is a good solution. It assumes users select multiple types per element; which I don't think holds true. Use cases mostly cover specific modifications on one element. It works for breakpoints, as managing multiple breakpoints for files is normal.

digitarald commented 4 years ago

I even have some doubts that the hierarchical layout is a good solution.

Clarifying: I think one benefit of hierarchy is that repeated references to the same element are otherwise not easy to tell apart when the elements have similar class names. Grouping multiple checkboxes with the same element solves this.

Case in point from grid examples:

image
digitarald commented 4 years ago

(This is mostly captured in the PRD Milestone 1)

Adding to the scope of this initial UX work:

When opening the panel the first time, I want to have guidance on how to add breakpoint, so that I know what to do next to get started.

Considerations

violasong commented 4 years ago

If it's true that people usually won't want to add multiple types of breakpoints to one node, then seems like the Chrome solution would be the simplest to work with. Showing the other options feels potentially frustrating (e.g. deactivating a breakpoint, and then briefly needing to remember which of the checkboxes you want to re-activate). I'm assuming folks usually don't need to change the type of breakpoint for a node either. So it would look like:

[x] ul.examples-detail-list 
    Node Removal

[x] ul.examples-detail-list
    Subtree modification

[x] nav#super-nav
    Node Removal

[ ] div.some-other.stuff
    Attribute modification
violasong commented 4 years ago

Shared panel between Debugger & Inspector

Sounds good to have a shared experience. Easiest option is to add a Breakpoints tab to the Inspector third pane tab bar (perhaps between Fonts and Animations?). Crowded, but seems reasonable enough for now.

(After Unified Layout becomes real, active breakpoints can be surfaced in the default tab 🙏)

cc: @martinbalfanz

violasong commented 4 years ago

I'll add Blank State to my todo list, in addition to the gutter icons. Anything else I need to work on?

digitarald commented 4 years ago

Thank you @violasong!

If it's true that people usually won't want to add multiple types of breakpoints to one node …

Can you help be understand where we called this out before?

Showing the other options feels potentially frustrating (e.g. deactivating a breakpoint, and then briefly needing to remember which of the checkboxes you want to re-activate).

Maybe I am not understanding what would cause frustration here. The various types are acting in very different ways. It also helps that the initial idea was to not show all checkboxes for all files but only the ones ever used; so the user see only options that got used before.

So it would look like:

I am OK with this approach, as it bring UX parity with existing tools – but …

it feels like the conservative option. For example, I add one idea to my mockup that makes the Inspector DOM BP panel more powerful by providing options to add DOM BPs for the currently selected element – these would somewhat depend on the hierarchical layout:

image

violasong commented 4 years ago

If it's true that people usually won't want to add multiple types of breakpoints to one node …

Can you help be understand where we called this out before?

I was referring to this quote: "I even have some doubts that the hierarchical layout is a good solution. It assumes users select multiple types per element; which I don't think holds true. Use cases mostly cover specific modifications on one element."

Showing the other options feels potentially frustrating (e.g. deactivating a breakpoint, and then briefly needing to remember which of the checkboxes you want to re-activate).

Maybe I am not understanding what would cause frustration here. The various types are acting in very different ways. It also helps that the initial idea was to not show all checkboxes for all files but only the ones ever used; so the user see only options that got used before.

My (uncertain) assumption is that the main thing folks want to do with an existing breakpoint is disabling it and re-enabling it. If that is the case, unchecking one of three options, and needing to recheck that same one of three options, is more difficult than if there's just the one checkbox per breakpoint.

it feels like the conservative option. For example, I add one idea to my mockup that makes the Inspector DOM BP panel more powerful by providing options to add DOM BPs for the currently selected element – these would somewhat depend on the hierarchical layout:

I'm not necessarily opposed to this if my other concerns are less important than having a more powerful UI. With a full Inspector 3rd pane tab, we'd have plenty of room for it.

digitarald commented 4 years ago

@violasong @darkwing landed the first prototype for the panel.

image

The screenshot above already uses a different reps formatting, to avoid showing the while DOM node code.

I'd assume we want the output more like Grid pane's formatting, for color and font-family. Any other input?

digitarald commented 4 years ago

@violasong @darkwing landed the first prototype for the panel.

image

The screenshot above already uses a different reps formatting, to avoid showing the while DOM node code.

I'd assume we want the output more like Grid pane's formatting, for color and font-family. Any other input?

digitarald commented 4 years ago

Milestone 1 is shipping, I'll open M2 as separate bug.

This is what we ended up with:

image