1317076 - Make navigation marker more obvious

Status: VERIFIED FIXED

(DevTools :: Console, enhancement, P4)

Description

[Tim Nguyen :ntim]

The new navigation marker doesn't stand out at all, and is easy to dismiss by scrolling through messages.

Comment 1

[Brian Grinstead [:bgrins]]

We should also make sure they don't get filtered out by turning off the 'log' filter (treat them like jsterm evaluations)

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

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.

Comment 3

[Siddharth Maurya [:savvysiddharth]]

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

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

(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.

Comment 5

[Siddharth Maurya [:savvysiddharth]]

Thank you sir. This info is very helpful. I'll go through all related files and work on changes.

Comment 6

[Siddharth Maurya [:savvysiddharth]]

Attachment: 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.

Comment 7

[Siddharth Maurya [:savvysiddharth]]

Attachment: screenshot of console for light and dark theme

Comment 8

[Siddharth Maurya [:savvysiddharth]]

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

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Thanks a lot Siddharth !
I assigned the bug to you and I'm going to review it right now.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b684eb7a73c0
Making navigation marker more obvious; r=nchevobbe

Comment 11

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/b684eb7a73c0

Comment 12

[Nicolas Chevobbe [:nchevobbe]]

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?

Comment 13

[Siddharth Maurya [:savvysiddharth]]

(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.

Comment 14

[Daniel Bodea [:danibodea]]

Hi Nicolas,

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

Thank you!

Comment 15

[Nicolas Chevobbe [:nchevobbe]]

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 !

Comment 16

[Daniel Bodea [:danibodea]]

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?

Comment 17

[Nicolas Chevobbe [:nchevobbe]]

(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.

Comment 18

[Daniel Bodea [:danibodea]]

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.