Reflect disabled breakpoint state in breakpoint list and gutter

RESOLVED FIXED in Firefox 68

Status

enhancement
P2
normal
RESOLVED FIXED
2 months ago
16 days ago

People

(Reporter: Harald, Assigned: jarilvalenciano)

Tracking

(Blocks 1 bug)

Trunk
Firefox 68

Firefox Tracking Flags

(firefox68 fixed)

Details

(URL)

Attachments

(5 attachments)

(Reporter)

Description

2 months ago

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 :/ .

Good idea!

Priority: -- → P2

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

Blocks: dbg-frontend
No longer blocks: dbg-column-bps-m1
Assignee: nobody → jarilvalenciano
Duplicate of this bug: 1542127
(Assignee)

Comment 4

a month ago

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

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

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.

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.)

(Assignee)

Comment 8

a month ago

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.

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.

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.

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.

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".

#2 looks pretty good to me

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

a month ago
Pushed by dwalsh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bf3af9a319ee
Reflected the disabled breakpoint state in breakpoint list and gutter r=davidwalsh,flod

Comment 16

a month ago
bugherder
Status: NEW → RESOLVED
Last Resolved: a month ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
You need to log in before you can comment on or make changes to this bug.