Closed Bug 1382184 Opened 2 years ago Closed 2 years ago

Improve colors of DevTools Notification Bar

Categories

(DevTools :: Shared Components, enhancement, P3)

enhancement

Tracking

(firefox57 fixed)

VERIFIED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: Honza, Assigned: abhinav.koppula, Mentored)

References

Details

Attachments

(3 files)

This is a follow up for bug 1304837

Notification Bar colors (text, background, borders, icons) should be revised to make sure that these colors work across supported OSes and DevTools themes. They should also be consistent with other UI DevTools widgets.

Honza
Screenshots of the current state (Win10, OSX, Linux) attached.

Honza
The notification bar close icon [messageCloseButton] should have a filter that inverts the cross icon depending on themes. It seems almost invisible on dark theme.

See also this comment:
https://bugzilla.mozilla.org/show_bug.cgi?id=1304837#c26

Honza
See Also: → 1382187
Priority: -- → P3
Severity: normal → enhancement
Assignee: nobody → 3ugzilla
Mentor: odvarko
Status: NEW → ASSIGNED
Some pieces of the NotificationBar has already been fixed (e.g. bug 1382187 fixed the icon).

The remaining thing seems to be the text color.

It's currently gray in both light and dark themes 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 color.

(you might also see my comment in bug 1382187 comment #8)

Honza
Hi Honza,
This looked like a small fix to me. I have created a mozreview-request. 
Please let me know if this was what you were looking for.
Comment on attachment 8910399 [details]
Bug 1382184 - Change text color of notification-box to 'theme-toolbar-color'.

https://reviewboard.mozilla.org/r/181838/#review187388

Works for me (tested on Win10 and OSX)

R+, thanks!

Honza
Attachment #8910399 - Flags: review?(odvarko) → review+
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/870ce315712c
Change text color of notification-box to 'theme-toolbar-color'. r=Honza
I'm not sure why this wasn't mentioned by Honza but please note that it is not considered good practice to "steal" bugs from assignees without requesting a needinfo and I'd hate to see that happen more in the future. It creates an environment of competition and urgency that drives people away (not to mention the immediate disappointment of getting your bug taken away).

Thank you.
Assignee: 3ugzilla → abhinav.koppula
(In reply to Johann Hofmann [:johannh] from comment #10)
> I'm not sure why this wasn't mentioned by Honza but please note that it is
> not considered good practice to "steal" bugs from assignees without
> requesting a needinfo and I'd hate to see that happen more in the future. It
> creates an environment of competition and urgency that drives people away
> (not to mention the immediate disappointment of getting your bug taken away).
> 
> Thank you.

Ah, sorry, that's my mistake! I accidentally assigned the bug to wrong email '3ugzilla@gmail.com' [:Towkir] (previously working on related bug) while I should use this e-mail address: abhinav.koppula@gmail.com (upon our prior offline discussion). So, no steeling happened in this case. Sorry about that.

Honza
(In reply to Jan Honza Odvarko [:Honza] from comment #11)
> Ah, sorry, that's my mistake! I accidentally assigned the bug to wrong email
> '3ugzilla@gmail.com' [:Towkir] (previously working on related bug) while I
> should use this e-mail address: abhinav.koppula@gmail.com (upon our prior
> offline discussion). So, no steeling happened in this case. Sorry about that.
About this, I am pretty sure we talked yesterday over IRC and you assigned this to me, it was unassigned for a long time and we decided to clear it. As a small change, I could do it fast, but things came up with this and I asked questions to you on IRC but you were offline, and I waited.

Guys, it's totally fine for me if someone wants to work on bugs, I would rather help if I can. But from new contributors I would expect them to abide by the Bugzilla Etiquette[0] and ask people on bugs before working on that.
It would be nice if I could hear from :Abhinav regarding this.

Let's now talk about why I waited before submitting my patch:

As we are talking about the improvement of the devtools notification bar,
there are three types of notification bars, "info", "warning" and "critical" [code resides at [1]]
I think this is the bug where we take care of them all, what do you say honza ?
As patch is now landed, we might need a new bug.
[I asked this on IRC, but you were offline]

I guess Abhinav can work on the new bug too :)

Cheers !

[0] https://bugzilla.mozilla.org/page.cgi?id=etiquette.html
[1] https://dxr.mozilla.org/mozilla-central/source/devtools/client/shared/components/notification-box.css#35-58
Flags: needinfo?(odvarko)
Flags: needinfo?(abhinav.koppula)
(In reply to [:Towkir] Ahmed from comment #12)
> (In reply to Jan Honza Odvarko [:Honza] from comment #11)
> > Ah, sorry, that's my mistake! I accidentally assigned the bug to wrong email
> > '3ugzilla@gmail.com' [:Towkir] (previously working on related bug) while I
> > should use this e-mail address: abhinav.koppula@gmail.com (upon our prior
> > offline discussion). So, no steeling happened in this case. Sorry about that.
> About this, I am pretty sure we talked yesterday over IRC and you assigned
> this to me, it was unassigned for a long time and we decided to clear it. As
> a small change, I could do it fast, but things came up with this and I asked
> questions to you on IRC but you were offline, and I waited.
> 
> Guys, it's totally fine for me if someone wants to work on bugs, I would
> rather help if I can. But from new contributors I would expect them to abide
> by the Bugzilla Etiquette[0] and ask people on bugs before working on that.
> It would be nice if I could hear from :Abhinav regarding this.

I think we established that this was a harmless mistake from Honza and there is no need to dwell on it or put Abhinav on the spot for the mistake of someone else. Please be considerate and assume good intentions.

Leaving needinfo on Honza for the technical questions.
Flags: needinfo?(abhinav.koppula)
(In reply to [:Towkir] Ahmed from comment #12)
> (In reply to Jan Honza Odvarko [:Honza] from comment #11)
> > Ah, sorry, that's my mistake! I accidentally assigned the bug to wrong email
> > '3ugzilla@gmail.com' [:Towkir] (previously working on related bug) while I
> > should use this e-mail address: abhinav.koppula@gmail.com (upon our prior
> > offline discussion). So, no steeling happened in this case. Sorry about that.
> About this, I am pretty sure we talked yesterday over IRC and you assigned
> this to me, it was unassigned for a long time and we decided to clear it. As
> a small change, I could do it fast, but things came up with this and I asked
> questions to you on IRC but you were offline, and I waited.
Yeah, I messed it up I was chatting with both of you yesterday and
mixed who is working on what.

> As we are talking about the improvement of the devtools notification bar,
> there are three types of notification bars, "info", "warning" and "critical"
> [code resides at [1]]
> I think this is the bug where we take care of them all, what do you say
> honza ?
> As patch is now landed, we might need a new bug.
> [I asked this on IRC, but you were offline]
And this is correct I filled bug 1401948

> I guess Abhinav can work on the new bug too :)
Abhinav, do you have time to work on that one?

---

Also, you guys are both doing great work on DevTools and not only me
but others from the DevTools team appreciate that!

Honza
Flags: needinfo?(odvarko)
https://hg.mozilla.org/mozilla-central/rev/870ce315712c
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
> > I guess Abhinav can work on the new bug too :)
> Abhinav, do you have time to work on that one?
> 
Yes, Honza would be glad to work on it.
I have successfully reproduced this bug with Nightly 56.0a1 (2017-07-19) (32-bit)  on windows 10(32bit)

this bug is verified fix with  latest beta 57.0 (32-bit)
Build ID: 20171106194249
Mozilla/5.0 (Windows NT 10.0; rv:57.0) Gecko/20100101 Firefox/57.0

[bugday-20171108]
QA Whiteboard: [bugday-20171108]
I have reproduced this bug with Nightly 56.0a1 (2017-07-19) on Ubuntu 16.04 LTS!

This bug is verified as fixed with latest Beta !

Build  ID :  20171102181127
Use Agent :  Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0

[bugday-20171108]
As this bug is verified in both linux(comment 17) and windows (comment 18), I am marking this bug as verified fixed.
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.