Closed
Bug 1421491
Opened 7 years ago
Closed 6 years ago
Response headers: minor style fixes needed for selected row, hovered accordion header
Categories
(DevTools :: Netmonitor, defect, P3)
DevTools
Netmonitor
Tracking
(firefox59 fixed)
RESOLVED
FIXED
Firefox 59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: victoria, Assigned: sagarbharadwaj50)
Details
Attachments
(2 files, 2 obsolete files)
This could go well with some of the Netmonitor visual polish work for Q1. Image that shows the bugs attached: - Selected rows with deep blue background should have white text (the magenta is too hard to read). Right now, if you click the key there's no magenta, but clicking the value or elsewhere in the row causes it to be magenta. - Accordion header should not turn blue or have underline when hovered
Reporter | ||
Comment 1•7 years ago
|
||
Oops - here's a new image that actually shows both problems
Updated•7 years ago
|
Attachment #8932679 -
Attachment is obsolete: true
Comment 3•7 years ago
|
||
UI Analysis doc updated. Honza
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•7 years ago
|
||
Hello, I have submitted a mozreview-request for this bug. Please review. Apologies if I have unknowingly broken anything.
Comment hidden (mozreview-request) |
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8936166 [details] Bug 1421491 - Style fixes in Netmonitor https://reviewboard.mozilla.org/r/206928/#review212802 Thanks for the patch! Please see my inline comments Honza ::: devtools/client/netmonitor/src/assets/styles/NetworkDetailsPanel.css:144 (Diff revision 1) > > .network-monitor .tree-container .treeTable .treeValueCell input{ > color: var(--string-color); > } > > +.treeValueCell input:focus { Please use more specific selector: .network-monitor .tree-container .treeTable .treeValueCell input:focus ::: devtools/client/shared/components/tree/TreeView.css:76 (Diff revision 1) > } > > .treeTable .treeRow.selected { > + color: var(--theme-selection-color); > background-color: var(--theme-selection-background); > } Both changes in this file should go to NetworkDetailsPanel.css and they should also use more precise selector like in the previous case.
Attachment #8936166 -
Flags: review?(odvarko) → review-
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8936168 [details] Bug 1421491 - Style fixes in Netmonitor https://reviewboard.mozilla.org/r/206930/#review212804 Please merge this patch with the first one. Honza
Attachment #8936168 -
Flags: review?(odvarko) → review-
Updated•7 years ago
|
Assignee: nobody → sagarbharadwaj50
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8936168 -
Attachment is obsolete: true
Assignee | ||
Comment 10•7 years ago
|
||
Hello Honza, I have updated my patch with the required changes. I have squashed everything to one commit.
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8936166 [details] Bug 1421491 - Style fixes in Netmonitor https://reviewboard.mozilla.org/r/206928/#review213140 Looks good, thanks! R+ Honza
Attachment #8936166 -
Flags: review?(odvarko) → review+
Assignee | ||
Comment 12•6 years ago
|
||
Hello Honza, Thanks for your review. As this is my first patch, I'm not really sure where to go from here. Can you push it to the try server for testing?
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Comment 13•6 years ago
|
||
Autoland can't push this patch until all pending issues in MozReview are marked as resolved. Please take care of those and then re-request checkin. Thanks!
Flags: needinfo?(sagarbharadwaj50)
Keywords: checkin-needed
Comment 14•6 years ago
|
||
Pushed by jodvarko@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9cd0e864b613 Style fixes in Netmonitor r=Honza
Comment 15•6 years ago
|
||
(In reply to Sagar Bharadwaj from comment #12) > Hello Honza, > > Thanks for your review. > As this is my first patch, I'm not really sure where to go from here. Can > you push it to the try server for testing? Sure, the try push is here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2799f2af1b21 (there is one failure, but unrelated) I also resolved both issues in MozReview and landed the patch. You should read about Review board here: http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html Thanks! Honza
Flags: needinfo?(sagarbharadwaj50)
Comment 16•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9cd0e864b613
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•