firefox-devtools / profiler

Firefox Profiler — Web app for Firefox performance analysis
https://profiler.firefox.com
Mozilla Public License 2.0
1.19k stars 389 forks source link

Callstack navigation: automatically expand to next branching node #517

Open violasong opened 7 years ago

violasong commented 7 years ago

Navigating the callstack seems slow since it requires two right-arrow key presses to expand and go to next row.

We should change it so that right-arrow (or clicking on disclosure triangle) automatically expands to the next node that has 2 or more children. It should auto-select that node.

Edit: When using left-arrow key or closing the arrow, it should have the opposite behavior - closing all nodes since the last fork.

I think it's possible that expanding all the way to the next branching node will feel like too much. The subtler version would be to just have each right-arrow key press trigger both expand and go to next row.

┆Issue is synchronized with this Jira Task

julienw commented 6 years ago

From Marco on Bugzilla, this can be a problem for accessibility as the controls would be different to what a tree normally is.

Possible solutions (?):

  1. Not doing this patch at all. Maybe https://github.com/devtools-html/perf.html/issues/682 will make this fast enough.
  2. stop exposing this interface as a tree, but add better titles and alt descriptions. Especially, in the patch I started in https://github.com/devtools-html/perf.html/pull/803, we expand until the next branching node, and it can be challenging for a screen reader user to understand what's going on given we skip a lot of nodes.
  3. doing this behavior only when another key is pushed (shift ? alt ?). Problem of discoverability.
  4. not worry about accessibility for now.

I think I'd settle for solution 3 for now, as it's easier and doesn't impair accessibility. For discoverability we could display a note if we detect that the user keeps the arrow key pressed.

MarcoZehe commented 6 years ago

This will introduce potentially confusing situations for users of assistive technologies. When a blind user, for example, hits RightArrow on a collapsed node, the screen reader will announce that the node has now been expanded. If a focus change happened immediately as a result of pressing RightArrow, this state change announcement would be nuked by the information being spoken as a result of the focus change. Moreover, if I understand the above correctly, it wouldn't even be the first child that gets focus, but any of a number of children, adding even more unpredictability to the behavior. For somebody who has to work sequentially and doesn't have the benefit of a broad overview, this will add a massive amount of cognitive load.

Therefore, I strongly must object to this change.

violasong commented 6 years ago

Thanks Marco, I appreciate learning about the accessibility issues for this - I had some sense that it was risky UX, but didn't know about these additional problems. Julien's idea of only doing this behavior with a modifier key seems like a good idea.

julienw commented 6 years ago

I moved the ticket back to the "todo" column. I think we still need this to improve the life of our users, just we need to use a modifier like shift or ctrl for accessibility reason, and find a way to make it discoverable by our users. I think most of the code I did in https://github.com/devtools-html/perf.html/pull/803 still works.

fqueze commented 5 years ago

As an alternative to the key modifier, when expanding is triggered by a click, could we expand automatically to the next branching when there's more than eg. 500ms between the mouse down and mouse up events?

julienw commented 5 years ago

I feel it's even less discoverable :/