Closed
Bug 1429296
Opened 6 years ago
Closed 6 years ago
Show MDN link of filter properties when hover the filterbox
Categories
(DevTools :: Netmonitor, enhancement, P2)
DevTools
Netmonitor
Tracking
(firefox61 fixed)
RESOLVED
FIXED
Firefox 61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: gasolin, Assigned: Honza)
References
()
Details
Attachments
(2 files)
follow design in https://mozilla.invisionapp.com/share/2XEEY0RYA#/screens/263401584_Network_Toolbar Show MDN link of filter properties when user hover the filterbox
Reporter | ||
Updated•6 years ago
|
Assignee: nobody → gasolin
Status: NEW → ASSIGNED
Priority: -- → P2
Assignee | ||
Updated•6 years ago
|
Assignee: gasolin → nobody
Status: ASSIGNED → NEW
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → odvarko
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment 2•6 years ago
|
||
mozreview-review |
Comment on attachment 8971553 [details] Bug 1429296 - Show MDN link of filter properties when hover the filterbox; https://reviewboard.mozilla.org/r/240308/#review246232 Added a few comments, none of which should be blockers. ::: devtools/client/shared/components/MdnLink.js:18 (Diff revision 1) > - > -function MDNLink({ url }) { > return ( > a({ > className: "devtools-button learn-more-link", > - title: LEARN_MORE, > + title: title, Do we need the ":title"? Can it just be `title,`? ::: devtools/client/netmonitor/src/components/Toolbar.js:39 (Diff revision 1) > // Localization > const SEARCH_KEY_SHORTCUT = L10N.getStr("netmonitor.toolbar.filterFreetext.key"); > const SEARCH_PLACE_HOLDER = L10N.getStr("netmonitor.toolbar.filterFreetext.label"); > const TOOLBAR_CLEAR = L10N.getStr("netmonitor.toolbar.clear"); > const TOOLBAR_TOGGLE_RECORDING = L10N.getStr("netmonitor.toolbar.toggleRecording"); > +const LEARN_MORE_TITLE = L10N.getStr("netmonitor.headers.learnMore"); "Learn more" doesn't seem very informative -- learn more about what? Canw e make this message more descriptive, like "Learn more about Net Monitor"?
Attachment #8971553 -
Flags: review?(dwalsh) → review+
Comment 3•6 years ago
|
||
The mockups show the (?) as light grey, but the patch makes the (?) fairly dark, almost darker than every other bit of text (could just be the SVG width). Should we lighten the (?) or is that a side effect of the mockups?
Flags: needinfo?(odvarko)
Assignee | ||
Comment 4•6 years ago
|
||
Thanks for the review David! (In reply to David Walsh :davidwalsh from comment #2) > > className: "devtools-button learn-more-link", > > - title: LEARN_MORE, > > + title: title, > > Do we need the ":title"? Can it just be `title,`? Fixed > > +const LEARN_MORE_TITLE = L10N.getStr("netmonitor.headers.learnMore"); > > "Learn more" doesn't seem very informative -- learn more about what? True, changed to: "Learn more about filtering" > The mockups show the (?) as light grey, but the patch makes the (?) fairly > dark, almost darker than every other bit of text (could just be the SVG > width). Should we lighten the (?) or is that a side effect of the mockups? Good call @Victoria: the MDN icon is grey on the mockup, but the existing MDN icons are dark (it's dark in Headers side panel when hovering over it and also dark in Statistics view). What color is the right one? Honza
Flags: needinfo?(odvarko)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(victoria)
Comment 6•6 years ago
|
||
(In reply to Jan Honza Odvarko [:Honza] from comment #4) > @Victoria: the MDN icon is grey on the mockup, but the existing MDN icons > are dark (it's dark in Headers side panel when hovering over it and also > dark in Statistics view). What color is the right one? I think that since the MDN icon is a secondary action that's inside an input field, it should be Gray-50 (like the icons in the main Firefox input bar) (I think this makes sense when this toolbar is white - I'll file a separate bug for tweaks)
Flags: needinfo?(victoria)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 9•6 years ago
|
||
(In reply to Victoria Wang [:victoria] from comment #6) > I think that since the MDN icon is a secondary action that's inside an input > field, it should be Gray-50 (like the icons in the main Firefox input bar) I see, fixed. Honza
Comment 10•6 years ago
|
||
Pushed by jodvarko@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/42347190c35a Show MDN link of filter properties when hover the filterbox; r=davidwalsh
Comment 11•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/42347190c35a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Updated•6 years ago
|
QA Whiteboard: [good first verify]
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•