871059 - The sorted states of request menu headers do not take up full height of toolbar.

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect)

Description

[Girish Sharma [:Optimizer]]

Attachment: patch v0.1

These headers are not pretended to be buttons on a toolbar, and thus should not have height less than the toolbar (which in itself is not pretended to be a toolbar, but a <thead> kind of thing).

The patch attaches makes it look much better.

Comment 1

[Girish Sharma [:Optimizer]]

Attachment: screenshot of sorted Filename Header with patch

Comment 2

[Girish Sharma [:Optimizer]]

Attachment: screenshot of sorted Filename Header without the patch

Comment 3

[Victor Porof [:vporof][:vp]]

When attached to the side, i'd like the network table header height to be smaller, because we don't need to match the sidebar's tabs height. Something like 24 px should be ok I think. You decide if this is followup material or prefer fixing it here.

Comment 4

[Girish Sharma [:Optimizer]]

Attachment: patch v0.2

Windows was off by 1 px height b/w tabbox tabs and toolbar. Fixed that.
When the sidebar is docked below the network requests, making the toolbar height to be 24 px.

Comment 5

[Girish Sharma [:Optimizer]]

Attachment: patch v0.2.1

Whoops. Forgot to remove the now invalid comment.

Comment 6

[Victor Porof [:vporof][:vp]]

Comment on attachment 748392 [details] [diff] [review]
patch v0.2.1

Review of attachment 748392 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/linux/devtools/netmonitor.css
@@ +43,5 @@
>  .requests-menu-header-button {
>    -moz-appearance: none;
>    background: none;
>    min-width: 12px;
> +  min-height: 31px; /* remaining 1px comes from border of the button */

OCD nit: please capitalize first letter and end with a period on all comments.

@@ +366,5 @@
>  
>  /* Responsive sidebar */
>  @media (max-width: 700px) {
> +  #requests-menu-toolbar {
> +    height: 24px;

I think it'd be a good idea to make this 26px on linux. Fonts are generally much bigger there.

@@ +370,5 @@
> +    height: 24px;
> +  }
> +
> +  .requests-menu-header-button {
> +    min-height: 23px; /* remaining 1px comes from border of the button */

..so 25px here.

Comment 7

[Girish Sharma [:Optimizer]]

Attachment: patch v0.3

Review comments addressed.
Carry forward r+

Comment 8

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Fx-Team&rev=4dbd432e7667
https://hg.mozilla.org/integration/fx-team/rev/781650c07e8c

Although: "CLOSED. BuiltBot issues: jobs don't seem to be showing up; tbpl fails to finish loading; retriggers not working;."

Comment 9

[Joe Walker [:jwalker]]

https://hg.mozilla.org/mozilla-central/rev/781650c07e8c