1507748 - Cursor should be set to "pointer" for clickable elements

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Martin Balfanz [:mbalfanz]]

In the flexbox sidebar, the cursor should be set to "pointer" for clickable elements.

Places I found where it's missing:
- Flex item -> parent back arrow
- Flex item sibling selector next to back arrow
- Flex item list, items

Comment 1

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 1507748.patch [1.0]

Comment 2

[Patrick Brosset <:pbro>]

Ok, maybe this isn't enough of a big deal to warrant this discussion, but I feel like I need to raise a point:

Today, we seem to have a very inconsistent way of using the pointer cursor vs. using the default cursor.

If I look at desktop applications I have running on my computed now, including Firefox and macOS Pages, I don't see the pointer cursor being used. Typically, desktop apps display buttons in a way that is easily recognizable by users (often using a different hover style, or borders and such), and just keep the default cursor.

My impression is that the pointer cursor is mapped to people's idea of a link. Often a piece of text that is underlined. And clicking on this will navigate you somewhere else.

DevTools, to me, fits in the desktop apps side, and for this reason uses a lot of buttons with hover styles and borders that are easily (hopefully) recognized by users as interactive, without needing to use the pointer cursor style.
DevTools also has links, which makes it a little special I guess. For instance it displays URL to resources in various places, and underlines them, and uses the pointer cursor to indicate that a link will be followed when the user clicks on them.

Now, as I said at the start, we aren't actually consistent with this. There are buttons in devtools to use the pointer cursor (e.g. flex/grid/event badges in the inspector, inspector icon in the rules-view, expand/collapse triangle icons, etc.). But there are also other places were we don't use this cursor (toolbar icons, tabs, overflow menu, etc.)

Does that mean we should do either one or the other? If so, which one? Should we have a rule for this?

Looking at Chrome DevTools quickly, the situation is very similar. On the surface at least, I couldn't see any logic for when the pointer and default cursors are used.

So, basically, the impact for this bug is that we should decide if we do the requested change for the flexbox panel, if we do it generally for DevTools, or if we stick to the default cursor everywhere.

Comment 3

[Patrick Brosset <:pbro>]

I was talking to Florens (:fvsch) on Slack just now and here's what they said:

"My advice would be to follow what Firefox UI does, and keep `cursor:default` for buttons. If it's unclear that an element can be clicked, adding `cursor:pointer` to it is a dirty fix for a design problem (lack of affordance). [...] I would use `cursor:pointer` only if it takes users somewhere, e.g. a link to a source file (and line number) in the Console, the "Show in inspector" buttons in Console, etc."

Comment 4

[Patrick Brosset <:pbro>]

Victoria, the code changes here are really simple so I can R+ this and Gabriel can land it really quickly. But based on comment 2 and comment 3, what would be your recommendation for this bug? Should we go ahead with the fix (which makes all buttons and toolbar-buttons have a pointer cursor in devtools, globally), or should we instead go back to the default cursor for everything that's not a link?

Comment 5

[Florens Verschelde :fvsch]

Also shortly discussed with Matt in https://github.com/devtools-html/ux/issues/24#issuecomment-432688555

Comment 6

[Martin Balfanz [:mbalfanz]]

I'd appreciate if we have UX guidelines on this.  I generally agree that the pointer should be used when "it takes users somewhere".  And isn't that what's happening here?  When a user clicks a flex-item in the sidebar, we are navigating to this flex-item, changing effectively everything in the inspector panel.  Does that not qualify because the user remains in the same panel?

Comment 7

[Florens Verschelde :fvsch]

> And isn't that what's happening here?

I believe so.

Since this action (and the back button when we're on a flex child) changes the focused node in the markup panel, and as a result changes all the content in the Layout panel and in other panels (Rules, Computed, Animations, and at least parts of Fonts), I do believe this example qualifies as a kind of "navigation".

Another example we could examine is the markup breadcrumbs:

1. The left/right scrolling buttons manipulate the view but do not switch to a different view: arrow cursor.
2. The breadcrumb item (except the current one) switch the user to a different node, changing the content in all other panels: hand cursor would be good here.

At least that's the theory. Sometimes things can be a bit murky and it's not obvious which call to make.

Comment 8

[Patrick Brosset <:pbro>]

Comment on attachment 9026194 [details] [diff] [review]
1507748.patch [1.0]

Review of attachment 9026194 [details] [diff] [review]:
-----------------------------------------------------------------

Based on the discussion on this bug, I'm cancelling the review. Could you add the pointer cursor only to the flexbox inspector buttons that do lead to navigation? (i.e. the flex item list, the back arrow).
This way this solves the original request without attempting to fix a global devtools-wide problem too hastily.

Comment 9

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 1507748.patch [2.0]

I am sending the review over to you Micah since pbro is on PTO tomorrow.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ad8114bf526c
Add a pointer cursor to the flexbox inspector buttons. r=mtigley

Comment 11

[Cristina Coroiu [:ccoroiu]]

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

Comment 12

[Victoria Wang [:victoria]]

Sorry for the late reply - yes, it would be great to figure out a DevTools-wide system later; meanwhile this patch for Flexbox sounds good to me.

Comment 13

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I can confirm this issue is fixed, I verified using Fx 65.0b10 on Windows 10 x64, macOS 10.13 and Ubuntu 16.04 LTS.
Cheers!