Closed Bug 1463780 Opened 6 years ago Closed 6 years ago

Use cursor types accordingly to the selectable nature of the element

Categories

(DevTools :: Inspector: Animations, defect, P3)

Product:
DevTools
Component:
Inspector: Animations
Type:
defect

Tracking

(firefox-esr52 unaffected, firefox-esr60 unaffected, firefox60 unaffected, firefox61 disabled, firefox62 verified)

Status:
VERIFIED FIXED
Milestone:
Firefox 62
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox60 --- unaffected
firefox61 --- disabled
firefox62 --- verified

People

(Reporter: aflorinescu, Assigned: daisuke)

References

(Blocks 2 open bugs)

Details

(Keywords: regression)

Attachments

(1 file)

Bug 1463780: Set background-color to inspection icon when hover so as to match old animation inspector.
59 bytes, text/x-review-board-request
gl
: review+
Details 
[Environment:]
Mac Osx 10.12
Ubuntu 16.04
Windows 10

Firefox 62.0a1 20180522220730

[Description:]
In the React Animation Inspector there are two types of pointer icons:
- 'arrow pointer' 
- 'hand pointer'

(in the old animation inspector there was also a 'text cursor pointer', but I think that the current React Animation is better off without it).

Currently the 'hand pointer' is present just on the hover over the animation graph, for the rest of Animation Inspector and Animation details, the 'arrow pointer' is used. 'Hand pointer' should be used for all the selectable items like: target icon, animation bar, animation node, (see below)

[Steps:]
1. Open in a new tab: https://rawgit.com/dadaa/3b73f847427025b51ba1ab7333013d0c/raw/77f3f0bb884875a179c3407f73bf8a8dd54751c9/doc_simple_animation.html
2. F12 / Inspector / Animations.
3. Select from the html panel the <body> and hover over the Animations element names in the Animations panel.
4. Click on any node from the Animations panel.
5. Hover over the element name opened at the above step.
6. Hover over the target icon.
7. Hover over the Script Animation details.
8. Hover over the Animation bar.

[Actual Result:]
3. 'arrow pointer'
4. The animation for the selected node is displayed (bug 1453614 fixed)
5. 'arrow pointer' 
6. 'arrow pointer'
7. 'arrow pointer'
8. 'hand pointer'

[Expected Result:]
3. 'hand pointer'
5. 'hand pointer'
6. 'hand pointer'
7. 'arrow pointer'
8. 'hand pointer'

[Note:] See old inspector pointer usage (FX61) use for further clarification.
Assignee: nobody → dakatsuka
Thank you very much, Adrian!

3, 5, 6 elements are using Rep component. Likewise, the other inspector as well are using this component. And the cursor of that as well is "arrow" even the component is clickable. So, for consistency, I don't know which is better.

You can confirm the behavior following steps:
1. Open https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Grid_Layout
2. Open inspector
3. Select "Layout"
4. Open "Grid" accordion
You can check the component following "Overlay Grid" text.

However the target(inspect) icon is different behavior. If mouse over, the color changed.
We may be able to introduce this behavior to animation inspector as well.
But, please let me ask to UX person once.

Victoria, sorry to bother you.
How do you think?
Flags: needinfo?(victoria)
Status: NEW → ASSIGNED
Priority: -- → P3
Hi! Yeah, there's a lot of inconsistency like this all over DevTools and I need to spend more time thinking about this at some point - for now, I would defer to what :digitarald says about best practices here: https://github.com/devtools-html/debugger.html/issues/6268#issuecomment-388422579). E.g. if it looks obviously interactive, arrow is fine, otherwise show the hand pointer. Thanks!
Flags: needinfo?(victoria)
Thank you for the helpful advice, Victoria!

I will change the cursor as Adrian suggested.
Comment on attachment 8981079 [details]
Bug 1463780: Set background-color to inspection icon when hover so as to match old animation inspector.

https://reviewboard.mozilla.org/r/247188/#review253348
Attachment #8981079 - Flags: review?(gl) → review+
This really should be going into the Reps module in GitHub. If you don't think this can be generalized in the Reps module, then this is ok.
Ah, indeed, it might be better for the consistency.
Okay, I'll modify the GitHub.
Thanks!
I have updated the CSS so as to match the old animation inspector behavior.
After the PR was merged into m-c, will land this as well.
Depends on: 1465547
Pushed by dakatsuka@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/017ae900fb0d
Set background-color to inspection icon when hover so as to match old animation inspector. r=gl
https://hg.mozilla.org/mozilla-central/rev/017ae900fb0d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox62: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Verified as fixed on Nightly 62.0a1(20180611100116) on Win10 x64, Ubuntu 12.04 and MacOS 10.12
The cursors are displayed according to expected result from description.
Status: RESOLVED → VERIFIED
status-firefox62: fixed → verified
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: