Make navigation marker more obvious

VERIFIED FIXED in Firefox 64

Status

P4
enhancement
VERIFIED FIXED
2 years ago
4 months ago

People

(Reporter: ntim, Assigned: savvysiddharth, Mentored)

Tracking

({good-first-bug})

unspecified
Firefox 64
good-first-bug

Firefox Tracking Flags

(firefox64 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
The new navigation marker doesn't stand out at all, and is easy to dismiss by scrolling through messages.
Priority: -- → P2
Flags: qe-verify+
Priority: P2 → P3
Whiteboard: [reserve-new-console]
QA Contact: iulia.cristescu
Whiteboard: [reserve-new-console] → [reserve-console-html]
We should also make sure they don't get filtered out by turning off the 'log' filter (treat them like jsterm evaluations)
Priority: P3 → P4
Whiteboard: [reserve-console-html]

Updated

a year ago
No longer blocks: 1308219
Severity: normal → enhancement

Updated

8 months ago
Product: Firefox → DevTools

Comment 2

5 months ago
There are some nice ideas in https://github.com/devtools-html/ux/issues/16 .
IMO, adding the "globe" icon and either a subtle grey background or a top-border would work nicely.
Mentor: nchevobbe
Keywords: good-first-bug
(Assignee)

Comment 3

5 months ago
I would like to work on this one, Can I know which files in devtools directory I should look for?

Comment 4

5 months ago
(In reply to Siddharth Maurya [:savvysiddharth] from comment #3)
> I would like to work on this one, Can I know which files in devtools
> directory I should look for?

Hello Siddharth !

First, you can have a look at https://docs.firefox-dev.tools/getting-started/ to set up the work environment (clone the repo, setup the tools, …).

Then, for this bug, you can check the "persist logs" checkbox in the console, go to another page to see the "navigation marker".
When it's done, you can use the browser toolbox (https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox) to inspect the console, see what the DOM looks like.

If you do that, you may noticed that there's no specific CSS class that makes a "navigation marker" stands out from other message. So we need to add this class.

The navigation message is defined here: https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/devtools/client/webconsole/utils/messages.js#198-205 , and again, there's nothing specific about it.

We should changed the `type` to something like NAVIGATION_MARKER (which does not exist yet, we should add it here https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/devtools/client/webconsole/constants.js#116-136).

Then, the type will be automatically added to the DOM because of this line https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/devtools/client/webconsole/components/Message.js#142 , where we retrieve the type and simply put it in the list of classes to add to the element.

That's step 1, we now have a distinct class we can target.

We now need to do step 2, which is, style this class.
For that, we use the webconsole.css file.
You can see here https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/devtools/client/themes/webconsole.css#167-190 that we already have rules for other icons. So after those, we could have another rule which would be like `.message.navigation-marker .icon` (if we named the type `navigation-marker`).

We need to add another rule for the top border.

For the images, you can see that we use variables, which are defined here : https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/devtools/client/themes/variables.css#216-219

So here we need to add a new variable for the navigation-marker icon, as well as an svg file for the icon.
The globe icon we want is available here https://design.firefox.com/icons/viewer/ , you can download it and put it in the devtools/client/themes/images/webconsole folder.

I hope this is helpful Siddharth, let me know if you have any question.
(Assignee)

Comment 5

5 months ago
Thank you sir. This info is very helpful. I'll go through all related files and work on changes.
(Assignee)

Comment 6

5 months ago
Created attachment 9012466 [details]
Bug 1317076 - Making navigation marker more obvious; r=nchevobbe

(Bug 1317076) Before this patch, in developer console, enabling persist log, message displayed "Navigated to" ..something was not distinguishable from console.log messages. Now specific class for navigation marker is added.
(Assignee)

Comment 7

5 months ago
Created attachment 9012471 [details]
screenshot of console for light and dark theme
(Assignee)

Comment 8

5 months ago
I've submitted patch and screenshot of changes made. Can you assign this bug to me?

Comment 9

5 months ago
Thanks a lot Siddharth !
I assigned the bug to you and I'm going to review it right now.
Assignee: nobody → savvysiddharth
Attachment #9012466 - Attachment description: Making navigation marker more obvious → Bug 1317076 - Making navigation marker more obvious; r=nchevobbe

Comment 10

4 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b684eb7a73c0
Making navigation marker more obvious; r=nchevobbe

Comment 11

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b684eb7a73c0
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox64: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64

Comment 12

4 months ago
Congrats Siddharth 🎉, the code landed in Firefox Nightly and will be available in Firefox 64 😎, thanks a lot!
If you want to, can you give me your twitter handle so I can credit you in the tweet I'll post about this update?
(Assignee)

Comment 13

4 months ago
(In reply to Nicolas Chevobbe from comment #12)
> Congrats Siddharth 🎉, the code landed in Firefox Nightly and will be
> available in Firefox 64 😎, thanks a lot!
> If you want to, can you give me your twitter handle so I can credit you in
> the tweet I'll post about this update?

Awesome :D !!
It is my honour to work with this community.
My twitter handle is @savvysiddharth
Thank you sir.
Hi Nicolas,

Can you please explain what and how I should verify about your implementation?

Thank you!
Flags: needinfo?(savvysiddharth)

Comment 15

4 months ago
Hello Daniel, 

With the following STR: 

1. Go to http://mozilla.org/
2. Open the console
3. Check the "persist logs" checkbox
4. Reload the page

You should see a "Navigated to" message. It should be blue and have a globe icon before.

Thanks !
Flags: needinfo?(savvysiddharth)
Hello again,

Your steps to reproduce work as expected, but the reason why I couldn't deduce the STR and the case that I was checking is not working as I was expecting:
1. Open a new tab.
2. Open the Web Console and check the "Persist Logs" checkbox.
3. Input any page URL into the address bar and tap Enter.
Expected: A message starting with the icon of a globe and the text "Navigated to <page URL>." is displayed.
Actual: The navigation marker is not displayed.

Is this implemented as intended or should it also display a message in this case?
Flags: needinfo?(savvysiddharth)

Comment 17

4 months ago
(In reply to Bodea Daniel [:danibodea] from comment #16)
> Hello again,
> 
> Your steps to reproduce work as expected, but the reason why I couldn't
> deduce the STR and the case that I was checking is not working as I was
> expecting:
> 1. Open a new tab.
> 2. Open the Web Console and check the "Persist Logs" checkbox.
> 3. Input any page URL into the address bar and tap Enter.
> Expected: A message starting with the icon of a globe and the text
> "Navigated to <page URL>." is displayed.
> Actual: The navigation marker is not displayed.
> 
> Is this implemented as intended or should it also display a message in this
> case?

My guess here is that you start with the "new page" which is different from reular pages. Which means, once you navigate from it, a "new" console is created (and not showing the navigated to" message),

So, an updated STR would be: 
1. Go to data:text/html,<meta charset=utf8>Hello
2. Open the console and check "persist logs"
3. Reload the page

You should see the message 

If you don't, then we have a problem.
Flags: needinfo?(savvysiddharth)
Hi,

The STR from comment 17 also works as intended, as it displays the message accordingly. 
If the that STR would be modified to open a new tab and open console before navigating to the snippet provided, the message is also not being displayed, but considering you say that this STR is sufficient, then this issue is now verified.

Thank you.
Status: RESOLVED → VERIFIED
status-firefox64: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.