1259475 - Remove hover state from disabled icon buttons

Status: RESOLVED FIXED

(DevTools :: Framework, defect, P2)

Description

[Tim Nguyen :ntim]

Example: the diff snapshots button in the memory tool.

Comment 1

[[:Towkir] Ahmed]

Hi Tim,
Can you please provide a screenshot, so that I can start with this ? 
Thanks

Comment 2

[Tim Nguyen :ntim]

(In reply to [:Towkir] Ahmed from comment #1)
> Hi Tim,
> Can you please provide a screenshot, so that I can start with this ? 
> Thanks

In the memory tool, the third button (the one that looks like [0]) is disabled, but when you hover it, you can see the opacity of the button change (it's subtle, but visible).


[0]: https://hg.mozilla.org/mozilla-central/raw-file/938ce16be25f9c551c19ef8938e8717ed3d41ff5/devtools/client/themes/images/diff.svg

Comment 3

[[:Towkir] Ahmed]

Thanks Tim, I will submit a patch soon, meanwhile you can have a look at the other one, may be mentor too if possible :)

Comment 4

[[:Towkir] Ahmed]

Hi Tim, does it look good on today's nightly ? 
I found this okay on linux.

Comment 5

[Tim Nguyen :ntim]

Still happening on macOS latest Nightly for me.

Comment 6

[Deepjyoti Mondal]

Attachment: bug1259475.patch

This is a test patch. I have added css rules to check whether the selected button is disabled or not. The diff-snapshot button does not change its opacity anymore on hovering mouse pointer over it when its disabled. hope this helps.

Comment 7

[Tim Nguyen :ntim]

Comment on attachment 8797570 [details] [diff] [review]
bug1259475.patch

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

::: devtools/client/themes/common.css
@@ +366,4 @@
>    opacity: 0.8;
>  }
>  
> +.devtools-button:hover:empty:not([disabled]):before,

:not(:disabled) is more standard. Can you use that?

Comment 8

[Deepjyoti Mondal]

Attachment: bug1259475.patch

I have updated my previous patch and replaced :not([disabled]) with :not(:disabled).

Comment 9

[Tim Nguyen :ntim]

Comment on attachment 8797611 [details] [diff] [review]
bug1259475.patch

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

::: devtools/client/themes/common.css
@@ +370,3 @@
>  .devtools-button[checked]:empty::before,
>  .devtools-button[open]:empty::before,
> +.devtools-toolbarbutton:not([label]):not(:disabled):hover > image,

So this (.devtools-toolbarbutton) should use :not([disabled=true]) while .devtools-button should use :not(:disabled)

Sorry if I wasn't clear enough.

Comment 10

[Deepjyoti Mondal]

Attachment: bug1259475.patch

Updated my previous patch.

Comment 11

[Tim Nguyen :ntim]

Comment on attachment 8798129 [details] [diff] [review]
bug1259475.patch

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

Thanks!

Comment 12

[Carsten Book [:Tomcat]]

has problems to apply:

patching file devtools/client/themes/common.css
Hunk #1 FAILED at 365
1 out of 1 hunks FAILED -- saving rejects to file devtools/client/themes/common.css.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh bug1259475.patch

Comment 13

[Deepjyoti Mondal]

Attachment: bug1259475.patch

I have updated my local repository. Hope this helps.

Comment 14

[Tim Nguyen :ntim]

Comment on attachment 8798353 [details] [diff] [review]
bug1259475.patch

You don't need to re-request review once you've been granted an r+

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/97d32293dec4
Remove hover state from disabled icon buttons. r=ntim

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/97d32293dec4

Comment 17

[Md Maruf Rahman]

I have reproduced this bug with Nightly 48.0a1 (2016-03-24) on Windows 7, 64 Bit !

This bug's fix is verified with latest Nightly

Build ID    20161109030210
User Agent  Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:52.0) Gecko/20100101 Firefox/52.0

 [bugday-20161109]

Comment 18

[Deepjyoti Mondal]

cool :)