Response headers: minor style fixes needed for selected row, hovered accordion header

RESOLVED FIXED in Firefox 59

Status

P3
normal
RESOLVED FIXED
a year ago
9 months ago

People

(Reporter: victoria, Assigned: sagarbharadwaj50)

Tracking

unspecified
Firefox 59

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

a year ago
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

a year ago
Oops - here's a new image that actually shows both problems
Thanks for the report Victoria!

Honza
Priority: -- → P3
UI Analysis doc updated.
Honza
Comment hidden (mozreview-request)
(Assignee)

Comment 5

a year 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 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 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-
Assignee: nobody → sagarbharadwaj50
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8936168 - Attachment is obsolete: true
(Assignee)

Comment 10

a year ago
Hello Honza,
I have updated my patch with the required changes. 
I have squashed everything to one commit.
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

a year 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

a year ago
Keywords: checkin-needed
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
(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

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9cd0e864b613
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59

Updated

9 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.