1486323 - Network monitor toolbar is 1px too high

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P3)

Description

[Florens Verschelde :fvsch]

Attachment: network-toolbar-height.png

For consistency with other parts of DevTools, the Network monitor toolbar (or toolbars, when splitted in 2) should be 28px high excluding borders.

Currently it’s 29px, 1px taller than the toolbox's tab bar, and 1px taller than the Console’s toolbar.

The issue comes from this code:

.devtools-toolbar-two-rows-1,
.devtools-toolbar-two-rows-2,
.devtools-toolbar-single-row {
  flex-grow: 1;
  min-height: var(--primary-toolbar-height);
  background-color: var(--theme-body-background);
}

The --primary-toolbar-height variable is set to 29px, which is intended for a toolbar which has a 1px border-bottom, leaving 28px for the content/padding area.
Here, only the .devtools-toolbar-two-rows-2 has a bottom border.

One solution is to give a border-bottom to each toolbar, and remove the border on `.devtools-toolbar-container`.

It might also be better to remove the `padding: 0 3px` on that container, since it makes for an incomplete border (probably on purpose, but not consistent with other tools which have full-width borders), and it leaves 3px of darker gray on each side.

I can probably make a patch.

Comment 1

[Florens Verschelde :fvsch]

Attachment: network-toolbar-background.png

It looks like this toolbar has a different background from most other toolbars. It uses the `--theme-body-background`, which is white in the light theme and Grey-80 (#2a2a2e) in the dark theme.

I would like to use the standard toolbar background instead (shown here in the light theme). It would be more consistent with the Inspector, Console and Storage tools, for instance.

Victoria, would that look like a good change to you?

Comment 2

[Victoria Wang [:victoria]]

Comment on attachment 9005176 [details]
network-toolbar-background.png

The white search bar/toolbar in light-mode Network is actually intended :) — there's a bug somewhere to change the Console to match it, and the Inspector will be getting this style of white rectangular search bar as well. It's a way to set this style of toolbar, with large flexible search input, apart from other types of toolbars, such as the tab bars.

I realize using Gray 80 for this is a bit inconsistent with the usual dark-to-light vs light-to-dark patterns for light/dark mode bars, but I feel it looks right in this case.

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

So, the background should stay as is, but we should fix the height.
Good instructions are already in comment #0

Honza

Comment 4

[shobhit.kr]

Hi is this assigned? I would like to pick this up if someone can guide me.. this is my first time contributing

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to shobhit.kr from comment #4)
> Hi is this assigned? I would like to pick this up if someone can guide me..
> this is my first time contributing

Assigned to you!

You can start with this MDN page:
https://developer.mozilla.org/en-US/docs/Tools/Contributing

Honza

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@shobhit: How is it going, do you need any help?

Honza

Comment 7

[Deepashree]

Hello! I'm looking to be assigned to a good first bug as well. Is @shobhit still assigned? Could I be assigned in case it becomes available?

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Deepashree from comment #7)
> Hello! I'm looking to be assigned to a good first bug as well. Is @shobhit
> still assigned? Could I be assigned in case it becomes available?
Yes, but let's give shobhit a few days to reply.
I no reply in 2 days, ping me I'll reassign it.

Honza

Comment 9

[shobhit.kr]

(In reply to Jan Honza Odvarko [:Honza] from comment #6)
> @shobhit: How is it going, do you need any help?
> 
> Honza

Hi -- sorry I got preoccupied with some personal work
I tried building firefox but I am stuck
I am trying to do it on Linux Mint 18.3 and this comes up "Only GCC 6.1 or newer is supported (found version 5.4.0)"
Running "gcc-6 --version" gives me this "gcc-6 (Ubuntu 6.4.0-17ubuntu1~16.04) 6.4.0 20180424"

I could see a related bug opened as well https://bugzilla.mozilla.org/show_bug.cgi?id=1444274
not sure if this is the same problem I am having?

Comment 10

[Florens Verschelde :fvsch]

Regarding the GCC issues: for working on CSS, you might not need to use gcc at all.
Did you run the `./mach bootstrap` command when getting started? If yes, did you pick the option to build Firefox for desktop with artifact builds?

Artifact builds use precompiled binaries and inject your CSS and JS changes. There should be no gcc involved. (But maybe the bootstrap command does try to check your gcc version in any case, so I can't guarantee it'll work.)

Comment 11

[shobhit.kr]

(In reply to Florens Verschelde from comment #10)
> Regarding the GCC issues: for working on CSS, you might not need to use gcc
> at all.
> Did you run the `./mach bootstrap` command when getting started? If yes, did
> you pick the option to build Firefox for desktop with artifact builds?
> 
> Artifact builds use precompiled binaries and inject your CSS and JS changes.
> There should be no gcc involved. (But maybe the bootstrap command does try
> to check your gcc version in any case, so I can't guarantee it'll work.)

Yeah, I got it working, I guess some files did not download properly. Now when I am trying to build I get this message
"gecko-dev/obj-x86_64-pc-linux-gnu/dist/system_wrappers/cstddef:3:15: fatal error: 'cstddef' file not found"

All the dependencies seemed fine when I ran mach build.. I am not able to launch firefox without having this build completed

Comment 12

[Shovan Chatterjee]

Hello, I'm looking to be assigned to a good first bug. Could I assigned, if its available?

Comment 13

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Shovan Chatterjee from comment #12)

Hello, I'm looking to be assigned to a good first bug. Could I assigned, if its available?

Done, thanks for the help!

Honza

Comment 14

[lloan alas:[lloanalas]]

Hi,

I'm an outreachy applicant. If this bug is still available, I'd be happy try working on it. Haven't seen any activity so figured I'd show my interest in working on it.

Thanks a ton!

Comment 15

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@Shovan: are you working on this?

Honza

Comment 16

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to lloan:[lloan] from comment #14)

I'm an outreachy applicant. If this bug is still available, I'd be happy try working on it. Haven't seen any activity so figured I'd show my interest in working on it.

Assigned to you, thanks!
Honza

Comment 17

[lloan alas:[lloanalas]]

Attachment: Test for Bug 1486323

I modified a bit more than the original suggestion pointed out. Even after doing what comment #0 suggested there was still a remainder of 0.8px on one of the rows. It seems that the search element height is static and set to 29px - went ahead and changed it to 28px as it also has a border. It still functions the same way and looks the same way as it did before and now the rows are the same size.

Comment 18

[lloan alas:[lloanalas]]

Attachment: Bug 1486323 - Network monitor toolbar is 1px too high. r=honza

Bug 1486323 - Network monitor toolbar is 1px too high. r=honza

Comment 19

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks for working on this!
Honza

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f2bd560573a4
Network monitor toolbar is 1px too high. r=Honza

Comment 21

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/f2bd560573a4

Comment 22

[lloan alas:[lloanalas]]

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #19)

Thanks for working on this!
Honza

Awesome - thanks a ton for the help :)!