Closed Bug 1487602 Opened 3 years ago Closed 3 years ago

Tracking icons is not visible when selected


(DevTools :: Netmonitor, defect, P3)



(firefox64 fixed)

Firefox 64
Tracking Status
firefox64 --- fixed


(Reporter: Honza, Assigned: kenokamo)



(Keywords: good-first-bug, Whiteboard: good-first-bug,)


(3 files, 3 obsolete files)

Attached image tracking-icon.jpg
Network panel is marking tracking resource using an icon (this feature has been introduced in bug 1333994).

This icon is almost invisible when the row is selected. See the attached screenshot.

The contrast between the icon and blue background should be high enough, so the icon is visible (the icon should be brighter).

Keywords: good-first-bug
Priority: -- → P3
Whiteboard: good-first-bug,
Duplicate of this bug: 1487913
Can I take this one?
Assigned to you.

Thanks for helping!
Assignee: nobody → kenokamo
Attached image screenshot (obsolete) —
I found adding a CSS selector with a light background improves the contrast a lot.

.selected .tracking-resource {
  background-color: #ececec;
  border-radius: 2px;

Let me know if you want to do it this way or have any ideas for improvements.
Attachment #9008791 - Attachment description: css updated background → screenshot
@kenokamo: thanks for working on this!

Please need info me next time so, your request isn't lost in bug mail.
Also, having regular patch attached would be better (it's not clear where you made that change).

Could we have the background transparent and the icon white if it's selected (just like the text color)?

Flags: needinfo?(kenokamo)
Attached image Screenshot - svg with #ffffff fill (obsolete) —
How does this look? 

I created a new file "shield-white.svg" which is the same as "shield.svg" except the fill color is #ffffff. In the CSS I replaced the previous changes I made with

.selected .tracking-resource {
  background-image: url(chrome://devtools/content/netmonitor/src/assets/icons/shield-white.svg);

The only problem is that it gets an error "Could not load the image" for the shield-white.svg.  Is there something I need to do to include the new file in the build process?

Also this is my first contribution using bugzilla.  Can you tell me how I make a patch file?  Thanks!
Attachment #9008791 - Attachment is obsolete: true
Flags: needinfo?(kenokamo) → needinfo?(odvarko)
The screenshot looks great!

Can we avoid adding a new icon?

E.g. using `filter: brightness(500%);` ?

Flags: needinfo?(odvarko) → needinfo?(kenokamo)
@kenokamo: any progress with the patch?

Here is what it looks like using the 500% brightness filter with the same icon.  I like it this way using the same file a lot better.
Attachment #9009195 - Attachment is obsolete: true
Flags: needinfo?(kenokamo) → needinfo?(odvarko)
Attached patch patch (obsolete) — Splinter Review
I couldn't find how to make the patch file in the contributing docs.  This is a patch I made by running 'hg diff -c tip > patch'.  Let me know if you need me to do it a different way.
Comment on attachment 9011584 [details] [diff] [review]

Review of attachment 9011584 [details] [diff] [review]:

Thanks for the patch!

Attachment #9011584 - Flags: review+
Comment on attachment 9011584 [details] [diff] [review]

Ah, the commit message needs to be set in the patch.
See this page:

Also don't forget to set the 'review' flag to `?` and my email address to make proper review request (and I can see it in my review queue)

Flags: needinfo?(odvarko)
Attachment #9011584 - Flags: review+
I looked through the developer guide and couldn't find the hg command to make a patch file with a commit message.  Can you tell me how I do it?  Thanks!
Flags: needinfo?(odvarko)
I am using HG queues:

# create new patch in the queue from changes in the working directory
hg qnew my-patch --message "Bug 1487602 - Tracking icon is not visible when selected; r=Honza"

# create patch
hg qtip export > my-patch.patch

# now you can pop the patch from the queue
hg qpop

# and remove when not needed anymore (be careful this deletes the changes)
hg qremove my-patch

Does that work for you?

Flags: needinfo?(odvarko) → needinfo?(kenokamo)
Attached patch 1487602v1.patchSplinter Review
Attachment #9011584 - Attachment is obsolete: true
Flags: needinfo?(kenokamo) → needinfo?(odvarko)
Attachment #9012640 - Flags: review?(odvarko)
Thanks.  I was able to use those steps to create a patch that includes the commit message.  Let me know if it is OK now.
Comment on attachment 9012640 [details] [diff] [review]

Review of attachment 9012640 [details] [diff] [review]:

Looks great to me, thanks!
Attachment #9012640 - Flags: review?(odvarko) → review+
The next step is to set `checkin-needed` keyword to make sure that somebody lands the patch in m-c (Mozilla Central) repository. I am doing it for now, so you can see it in action.

Thanks for the help!
Flags: needinfo?(odvarko)
Keywords: checkin-needed
Pushed by
Tracking icon is not visible when selected; r=Honza
Keywords: checkin-needed
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Thanks for all your guidance on this Honza!
You need to log in before you can comment on or make changes to this bug.