Tracking icons is not visible when selected

RESOLVED FIXED in Firefox 64

Status

P3
normal
RESOLVED FIXED
7 months ago
5 months ago

People

(Reporter: Honza, Assigned: kenokamo)

Tracking

({good-first-bug})

unspecified
Firefox 64
good-first-bug

Firefox Tracking Flags

(firefox64 fixed)

Details

(Whiteboard: good-first-bug,)

Attachments

(3 attachments, 3 obsolete attachments)

Posted 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).

Honza
Keywords: good-first-bug
Priority: -- → P3
Whiteboard: good-first-bug,
Duplicate of this bug: 1487913
(Assignee)

Comment 2

7 months ago
Can I take this one?
Assigned to you.

Thanks for helping!
Honza
Assignee: nobody → kenokamo
(Assignee)

Comment 4

6 months ago
Posted 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.
(Assignee)

Updated

6 months ago
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)?

Honza
Flags: needinfo?(kenokamo)
(Assignee)

Comment 6

6 months ago
Posted 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%);` ?

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

Honza
Status: NEW → ASSIGNED
(Assignee)

Comment 9

6 months ago
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)
(Assignee)

Comment 10

6 months ago
Posted 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]
patch

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

Thanks for the patch!

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

Ah, the commit message needs to be set in the patch.
See this page:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Creating_a_patch


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)

Thanks,
Honza
Flags: needinfo?(odvarko)
Attachment #9011584 - Flags: review+
(Assignee)

Comment 13

6 months ago
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?

Honza
Flags: needinfo?(odvarko) → needinfo?(kenokamo)
(Assignee)

Comment 15

6 months ago
Attachment #9011584 - Attachment is obsolete: true
Flags: needinfo?(kenokamo) → needinfo?(odvarko)
Attachment #9012640 - Flags: review?(odvarko)
(Assignee)

Comment 16

6 months ago
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]
1487602v1.patch

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

Looks great to me, thanks!
Honza
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!
Honza
Flags: needinfo?(odvarko)
Keywords: checkin-needed

Comment 19

6 months ago
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/71c746601621
Tracking icon is not visible when selected; r=Honza
Keywords: checkin-needed

Comment 20

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/71c746601621
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
status-firefox64: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
(Assignee)

Comment 21

6 months ago
Thanks for all your guidance on this Honza!
You need to log in before you can comment on or make changes to this bug.