Closed Bug 1444793 Opened 6 years ago Closed 6 years ago

Rework focus and selection highlighting in Developer tools tab bar

Categories

(DevTools :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox61 verified)

VERIFIED FIXED
Firefox 61
Tracking Status
firefox61 --- verified

People

(Reporter: birtles, Assigned: mantaroh)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(4 files)

Yura mentions: "it'd be fantastic to also put some UX effort in focus and selection highlighting. Right now we still resolve to dotted outline but it could be much better"
For reference, here's my focus ring doc which summarizes the issues throughout all of DevTools:
https://docs.google.com/document/d/1i74p_Tz3IEdX5Y1WjDNrG6WMRG7g6tT6SkUg2Z6lygw/edit#
This is the idea of focus ring:

https://docs.google.com/presentation/d/1V-j-bu8nlBW7z-bBJKLqMX0jFX8IIGJhunfu2g1eAmk/edit?usp=sharing

The change is that added the dotted border on the focused button. The width of the top of the border is 0px.

victoria:
What do you think about this change?
Flags: needinfo?(victoria)
We discussed with Victoria, then we need to remove dotted lines from focused style and change to the dark background that is the same as hover styles.
Flags: needinfo?(victoria)
Attached video focus-demo.mp4
This is the focus ring demonstration after discussing. I removed the dotted border from the focused tool tab button and apply dark background.
(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #4)
> Created attachment 8965618 [details]
> focus-demo.mp4
> 
> This is the focus ring demonstration after discussing. I removed the dotted
> border from the focused tool tab button and apply dark background.

Ah, I need to apply this style to another toolbox button as well. (e.g. picker / frame / ruler..etc)
Assignee: nobody → mantaroh
Status: NEW → ASSIGNED
Thank you Mantaroh! I looked at the demo and it looks great - very clean.
This is demonstration whish applying style to other buttons.
I applied this style to other devtools button(e.g. picker/ ruler/frames/close..etc), these button doesn't have a tab line which will display above of button. So I want to add this tab bar to these buttons. 

victoria,
Could you confirm this demonstration? I would like to get your feedback about this. If you need it, I want to work on it as another bug.
Flags: needinfo?(victoria)
(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #8)
> I applied this style to other devtools button(e.g. picker/
> ruler/frames/close..etc), these button doesn't have a tab line which will
> display above of button. So I want to add this tab bar to these buttons. 

I think I misunderstand. Do we really want to add the bar at the top to non-tab buttons? It sounds odd and I couldn't see it in the video.
(In reply to Brian Birtles (:birtles, travelling 7~18 April) from comment #9)
> (In reply to Mantaroh Yoshinaga[:mantaroh] from comment #8)
> > I applied this style to other devtools button(e.g. picker/
> > ruler/frames/close..etc), these button doesn't have a tab line which will
> > display above of button. So I want to add this tab bar to these buttons. 
> 
> I think I misunderstand. Do we really want to add the bar at the top to
> non-tab buttons? It sounds odd and I couldn't see it in the video.

Thanks, Brian.
I don't add tab line in non-tab buttons to this video yet. My concern is that it might be little difficult to find focused non-tab buttons compared to tab button. But it is probably just my subjective, So it is no problem to don't add this tab line.
Ok thanks. Yes, I suspect adding bars to the top would be confusing.
Thanks, Brian.
OK. I'll do that.
Flags: needinfo?(victoria)
(Just wanted to say: Yes, I can see the concern that the focused non-tab buttons can get a little lost with just a background color change - it's fine for now, but I'll keep this in mind for the full DevTools focus ring refresh.)
Comment on attachment 8966498 [details]
Bug 1444793 - Make focus style of tool tab to be same as hover style.

:nchevobbe putting you to review this since you recently did some focus-style reviews and I thought this might be related enough.
Attachment #8966498 - Flags: review?(gl) → review?(nchevobbe)
From your screencasts, this looks great to me :)
There's only one thing I fear, that we encountered in the debugger for the quicklist: 
if the mouse is already hovering a button and you navigates with the keyboard, you'd have 2 buttons which look the same, making it hard to know what will happen if you hit Enter (see attachment).
Maybe a plain blue border on focused would help ?
Comment on attachment 8966498 [details]
Bug 1444793 - Make focus style of tool tab to be same as hover style.

https://reviewboard.mozilla.org/r/235210/#review242564

Let's see what we can do to not have 2 buttons that look the same (and might confuse the user)

::: commit-message-83de5:1
(Diff revision 1)
> +Bug 1444793 - Make focus style of tool tab to be same as hover style. r?gl

change gl to nchevobbe

::: devtools/client/themes/common.css:736
(Diff revision 1)
> -  background: var(--tab-line-selected-color);
> +  background: var(--tab-line-selected-color) !important;
> +  opacity: 1;
> +  transform: scaleX(1);
> +}
> +
> +.devtools-tab:focus .devtools-tab-line {

instead of adding an !important to the previous rule, maybe we could write this one as `.devtools-tab:not(.selected):focus` ?
Attachment #8966498 - Flags: review?(nchevobbe) → review-
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #16)
> Created attachment 8968157 [details]
> hover_and_focused_similar
> 
> From your screencasts, this looks great to me :)
> There's only one thing I fear, that we encountered in the debugger for the
> quicklist: 
> if the mouse is already hovering a button and you navigates with the
> keyboard, you'd have 2 buttons which look the same, making it hard to know
> what will happen if you hit Enter (see attachment).

It indeed rings true. In that case, there are 2 same style buttons on a toolbar. I thought that user might not confuse since this effect is shown after occurring the user action immediately, but, as you mentioned, it is difficult to judge next behavior by looks.

> Maybe a plain blue border on focused would help ?

Do you mean blue bar of top? I have tried several patterns:
https://screenshots.firefox.com/JRDSFB3dGgLC52TE/null
(Image is "Selected" / "Focused" / "Hover" from left.)

I think it is easy to understand, if tab bar is blue.

Nicolas, Victoria,
As menstioned in comment 16, current focused tab and hover tab is same style, so it might bring some confusion for user. I tried 3 patterns.  What do you think about this styles?
Flags: needinfo?(victoria)
Flags: needinfo?(nchevobbe)
I'd like to hear Victoria's take on this. I think we discussed this at some point we discussed but decided it was ok.

(I suspect the chance of the mouse happening to hover over a button while navigating with the keyboard is slightly less for this case than the quick list in the debugger, but perhaps I'm just being optimistic!)
(In reply to Brian Birtles (:birtles, travelling 7~18 April) from comment #19)
> I'd like to hear Victoria's take on this. I think we discussed this at some
> point we discussed but decided it was ok.
> 
> (I suspect the chance of the mouse happening to hover over a button while
> navigating with the keyboard is slightly less for this case than the quick
> list in the debugger, but perhaps I'm just being optimistic!)

Yeah, you may be right. The quicklist issue happened because it appears in the middle of the screen, where you most likely hovered variables in the file. I'd be fine with the current patch, I just wanted to raise the issue
Flags: needinfo?(nchevobbe)
I think it would be fine to proceed with the current patch because keyboard navigation in this situation seems like it would be a pretty deliberate action that the user would be in control of, and in practice it wouldn't be that confusing.

However, would be interested in feedback from :yzen on this issue as well as the tab functionality in general - in Chrome I noticed that tabbing to the tabs and then using the arrow keys not only focuses on the tabs but selects them. If this would be the better behavior, then the need for a special focused non-selected tab style would go away.
Flags: needinfo?(victoria)
Thanks, Nicolas, Victoria, Birtles,

OK. The focused style will remain. i.e. I apply the gray tab bar style and gray background.
I've fixed the patch that was pointed out.
Comment on attachment 8966498 [details]
Bug 1444793 - Make focus style of tool tab to be same as hover style.

https://reviewboard.mozilla.org/r/235210/#review243262

One small nit, but looks good to me :)

::: devtools/client/themes/common.css:237
(Diff revision 2)
> +  padding-inline-end: 0px;
> +  padding-inline-start: 0px;

nit: it seems we use `0` in the file (and not `0px`)
Attachment #8966498 - Flags: review?(nchevobbe) → review+
> However, would be interested in feedback from :yzen on this issue as well as the tab functionality in general - in Chrome I noticed that tabbing to the tabs and then using the arrow keys not only focuses on the tabs but selects them. If this would be the better behavior, then the need for a special focused non-selected tab style would go away.

If we go this way, we would need to take care of performance. If someone want to go from inspector to style editor tab, and in the process enable both console and debugger, that might be an overkill
Comment on attachment 8966498 [details]
Bug 1444793 - Make focus style of tool tab to be same as hover style.

https://reviewboard.mozilla.org/r/235210/#review243262

Thank you.
I addressed it.
Pushed by mantaroh@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8b4050603e3d
Make focus style of tool tab to be same as hover style. r=nchevobbe
https://hg.mozilla.org/mozilla-central/rev/8b4050603e3d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
I have reproduced this issue using Firefox  60.0a1 (2018.03.08) on Ubuntu 14.04 x64.
I can confirm this issue is fixed, I verified using Firefox 61.0b9 on Win 10 x64, Mac OS X 10.13 and Ubuntu 14.04 x64.
Status: RESOLVED → VERIFIED
Depends on: 1466486
Product: Firefox → DevTools
Depends on: 1487107
You need to log in before you can comment on or make changes to this bug.