1537884 - Reflect disabled breakpoint state in breakpoint list and gutter

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement, P2)

Description

[:Harald Kirschner :digitarald]

Attachment: Disabled state in Chrome vs Firefox

Continue https://github.com/firefox-devtools/debugger/issues/6673

What were you doing?

1. Disable the Activate breakpoints button in Debugger

What happened?

No major UI changes, beyond the thin-lined button being blue.

What should have happened?

Breakpoints are clearly disabled everywhere they are shown, both the gutter and the list.

Anything else we should know?

Parity with Chrome. And not having it this clearly shown confused me already :/ .

Comment 1

[David Walsh :davidwalsh]

Good idea!

Comment 2

[Jason Laster [:jlast]]

Harald, is it alright if we move this to a more general quality meta?

Comment 4

[jaril]

Attachment: Bug 1537884 - Reflected the disabled breakpoint state in breakpoint list and gutter

Added new visual markers for the Breakpoints pane and the Editor's gutter to reflect the disabled breakpoint state.

Changes (to the disabled breakpoint state):

• removed syntax highlighting and faded all text to grey in the Breakpoints pane
• changed breakpoint marker colors in the gutter to grey
• changed column breakpoint marker colors in the gutter to grey

Image: https://user-images.githubusercontent.com/15959269/55636505-01961c00-5791-11e9-9f6d-84fbe153d55f.png

Comment 5

[Jason Laster [:jlast]]

I think I prefer chrome's approach of adding opacity. The grey colors feel a bit impersonal/cold to me.

Comment 6

[Florens Verschelde :fvsch]

I think I prefer chrome's approach of adding opacity. The grey colors feel a bit impersonal/cold to me.

Did you mean for the editor breakpoints, or in the Breakpoints pane?
When discussing our options with Jaril we found a bunch of issues with using opacity to suggest "breakpoints are deactivated".

Editor breakpoints:
Core problem is that we're trying to convey up to 3 bits of information for a given marker:

• Column markers: 1) deactivated (style?), 2) disabled (opacity: 0.6), 3) breakpoint opportunity (opacity: 0.3)
• Gutter markers: 1) deactivated (style?), 2) disabled (opacity: 0.6), 3) type (color variants)

We could use the same opacity: 0.6 for disabled and "deactivated" breakpoints, but then when users are in "deactivated" mode and change the enabled/disabled state of a given breakpoint they won't see a change in a UI at all.

Using greyscale markers was a way to avoid this issue, but it does lose the type information (color).
So it depends which trade-off we want.

Another option is to stop users from enabling/disabling breakpoints when in "deactivated" mode. Then we can mark all breakpoints as "disabled".

Breakpoints pane:

• Affordance issue: the overall low opacity (opacity: 0.3 or similar) like in Chrome suggests that the whole pane is disabled and can't be interacted with. Not quite: you can still interact with it normally. Tried it in Chrome, it feels really strange IMO.
• Accessibility: with partial opacity we end up way below WCAG contrast ratios.

Comment 7

[Florens Verschelde :fvsch]

Attachment: deactivated-breakpoint-marker-filter.png

Tried this style for the breakpoint markers:

svg {
  filter: grayscale(0.4) opacity(0.8);
}

(Left is normal style, right is with the filter applied; selector needs to be more precise, of course.)

Comment 8

[jaril]

Not a fan of disabling enabling/disabling breakpoints when in deactivated mode.

• The expectation from "Deactivate Breakpoints" is that breakpoints are simply skipped, similar to Chrome
• The breakpoint skipping behavior is in line with how we track it in the state (with skipPausing)
• It would require an additional step to toggle breakpoints in the Breakpoints Pane in the "Deactivate Breakpoints" case
• There doesn't seem to be a need to safeguard breakpoints from being toggled simply because breakpoints are being skipped

Having checkboxes in the disabled state would make the disabled state slightly more obvious, but I'm not sure it's worth changing the UX for.

I prefer the grey markers over the faded markers (with filter). It makes the markers and the syntax highlighted characters blend together, making it harder to figure out where the markers are.

From my experience, I'd rather have the deactivated state very obvious to avoid situations where the user might think breakpoints are broken instead of realizing that breakpoints are just being skipped.

Comment 9

[Florens Verschelde :fvsch]

I prefer the grey markers over the faded markers (with filter)

Tried your patch and I agree.
I find that the grayscale approach works well in practice, it really helps seeing the difference between the normal and "deactivated" states.

Comment 10

[Jason Laster [:jlast]]

I'm finding it a bit hard to follow this discussion and some of the trade-offs.

Disabled state UI

• We can show breakpoints in the gutter and editor in their disabled state.

  • It is okay if the user doesn't see which breakpoints were enabled/disabled individually here
  • The greyscale or even a greyscale filter could be confusing

• I like the overlay in the sidebar panel as it is clearly communicates we are disabled

  • This is an accessibility hit, but users should not be interacting with breakpoints in this mode.

Disabled state UX

• I like how chrome disables this mode when you add a breakpoint in the editor.
  • I think we should follow them here and add this behavior for the secondary panel as well.

Comment 11

[Florens Verschelde :fvsch]

Attachment: skip-pausing-alternatives1.png

Let's explore some color and opacity options then.

This series uses 0.6 opacity for the editor markers (using the "disabled" style for all breakpoints), and shows some possible variations for the Breakpoint pane.

Let me know what you think.

Comment 12

[Florens Verschelde :fvsch]

Attachment: skip-pausing-alternatives2.png

Here's an alternative style for the Editor breakpoint markers: in "skip pausing" mode, we can lower SVG's fill-opacity (to 0.15 in this example), making the markers look "hollow".

Comment 13

[Jason Laster [:jlast]]

#2 looks pretty good to me

Comment 14

[Florens Verschelde :fvsch]

Had a short meeting with Jason and David. Decision:

• Go with the look in #2 (attachment 9057034 [details]).
• Don't change anything to behavior yet (users can still enable/disable/remove/add individual breakpoints)

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf3af9a319ee
Reflected the disabled breakpoint state in breakpoint list and gutter r=davidwalsh,flod

Comment 16

[Noemi Erli[:noemi_erli]]

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