Closed
Bug 1317076
Opened 8 years ago
Closed 6 years ago
Make navigation marker more obvious
Categories
(DevTools :: Console, enhancement, P4)
DevTools
Console
Tracking
(firefox64 verified)
VERIFIED
FIXED
Firefox 64
Tracking | Status | |
---|---|---|
firefox64 | --- | verified |
People
(Reporter: ntim, Assigned: savvysiddharth, Mentored)
Details
(Keywords: good-first-bug)
Attachments
(2 files)
The new navigation marker doesn't stand out at all, and is easy to dismiss by scrolling through messages.
Updated•8 years ago
|
Priority: -- → P2
Updated•8 years ago
|
Flags: qe-verify+
Priority: P2 → P3
Whiteboard: [reserve-new-console]
Updated•8 years ago
|
QA Contact: iulia.cristescu
Updated•8 years ago
|
Whiteboard: [reserve-new-console] → [reserve-console-html]
Comment 1•7 years ago
|
||
We should also make sure they don't get filtered out by turning off the 'log' filter (treat them like jsterm evaluations)
Updated•7 years ago
|
Priority: P3 → P4
Updated•7 years ago
|
Whiteboard: [reserve-console-html]
Updated•7 years ago
|
No longer blocks: enable-new-console
Severity: normal → enhancement
Updated•6 years ago
|
Product: Firefox → DevTools
Comment 2•6 years 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•6 years ago
|
||
I would like to work on this one, Can I know which files in devtools directory I should look for?
Comment 4•6 years 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•6 years ago
|
||
Thank you sir. This info is very helpful. I'll go through all related files and work on changes.
Assignee | ||
Comment 6•6 years ago
|
||
(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•6 years ago
|
||
Assignee | ||
Comment 8•6 years ago
|
||
I've submitted patch and screenshot of changes made. Can you assign this bug to me?
Comment 9•6 years ago
|
||
Thanks a lot Siddharth !
I assigned the bug to you and I'm going to review it right now.
Assignee: nobody → savvysiddharth
Updated•6 years ago
|
Attachment #9012466 -
Attachment description: Making navigation marker more obvious → Bug 1317076 - Making navigation marker more obvious; r=nchevobbe
Comment 10•6 years ago
|
||
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b684eb7a73c0
Making navigation marker more obvious; r=nchevobbe
Comment 11•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Comment 12•6 years 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•6 years 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.
Comment 14•6 years ago
|
||
Hi Nicolas,
Can you please explain what and how I should verify about your implementation?
Thank you!
Flags: needinfo?(savvysiddharth)
Comment 15•6 years 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)
Comment 16•6 years ago
|
||
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•6 years 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)
Comment 18•6 years ago
|
||
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.
Description
•