Closed Bug 1317076 Opened 4 years ago Closed 2 years ago
Make navigation marker more obvious
The new navigation marker doesn't stand out at all, and is easy to dismiss by scrolling through messages.
Priority: P2 → P3
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)
No longer blocks: enable-new-console
Severity: normal → enhancement
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.
I would like to work on this one, Can I know which files in devtools directory I should look for?
(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.
Thank you sir. This info is very helpful. I'll go through all related files and work on changes.
(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.
I've submitted patch and screenshot of changes made. Can you assign this bug to me?
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
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/b684eb7a73c0 Making navigation marker more obvious; r=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?
(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!
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 !
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?
(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.
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.
You need to log in before you can comment on or make changes to this bug.