1382187 - Add a filter on the devtools notification bar close icon

Status: RESOLVED FIXED

(DevTools :: Shared Components, enhancement, P3)

Description

[[:Towkir] Ahmed]

Attachment: notification-bar-close-icon-dark-theme.png

[A follow up bug from bug 1304837 and bug 1382184]
The devtools notification bar close icon is almost invisible on dark theme. A filter should be added on this so that it can revert colors based on themes to be visible properly.

Comment 1

[[:Towkir] Ahmed]

Would you like to work on this ?

Comment 2

[Abhinav Koppula]

Attachment: Bug 1382187 - Theme specific filter added to devtools notification bar 'close' icon.

Review commit: https://reviewboard.mozilla.org/r/180684/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180684/

Comment 3

[Abhinav Koppula]

Hi Ahmed,
I have sent a mozreview-request for the above issue.
Was this what you were looking for?

Comment 4

[Abhinav Koppula]

Comment on attachment 8909084 [details]
Bug 1382187 - Theme specific filter added to devtools notification bar 'close' icon.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/180684/diff/1-2/

Comment 5

[Abhinav Koppula]

Hi Honza,
Have created a review-request for this.
Can you please review once?

Comment 6

[[:Towkir] Ahmed]

Hi Abhinav, I was about to bounce the review request to honza but first I wanted to check if your steps are correct and test things, but I could not manage time.
BTW, whenever you are working on a good-first bug, there should be some mentor on that bug and the mentor is usually the reviewer. Thus, honza is the reviewer of this bug. And it may take a bit of time to review sometimes (Assuming a reviewer may be busy sometimes)

Cheers!

Comment 7

[[:Towkir] Ahmed]

As you are making progress on it, I am assigning it to you.

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8909084 [details]
Bug 1382187 - Theme specific filter added to devtools notification bar 'close' icon.

https://reviewboard.mozilla.org/r/180684/#review186980

Thanks for the patch, tested on Win10 and OSX, works for me!

R+

It would be great to also fix the text of the notification bar.
It's currently gray in both light and dark thems and a bit hard to read.
I think it should be black in light theme and white in dark theme.

I believe we can just use: var(--theme-toolbar-color) for the text.
Can you please file a follow up.

Thanks!
Honza

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Jan Honza Odvarko [:Honza] from comment #8)
> It would be great to also fix the text of the notification bar.
> It's currently gray in both light and dark thems and a bit hard to read.
> I think it should be black in light theme and white in dark theme.
> 
> I believe we can just use: var(--theme-toolbar-color) for the text.
> Can you please file a follow up.
So, there is already bug filled for this, bug 1382184

Honza

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/905ddac6491c
Theme specific filter added to devtools notification bar 'close' icon. r=Honza

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/905ddac6491c