arni2033 Reporter
	Description • 8 years ago

>>>   My Info:   Win7_64, Nightly 48, 32bit, ID 20160330030326
STR:
1. Enable all tabs in devtools
2. Switch between Canvas, Performance, Memory, Network tabs

AR: In each tab clear button takes unique place
ER: Clear buttons should share the same position


Notes:
1) And also, probably, Netmonitor's toolbar shouldn't be so thick. No other toolbar has filter field
   with so big height. I think it's a part of this bug anyway.
2) This bug may be beyond my competence; probably somebody is already taking care of it somewhere,
   but I'm better not to risk letting this bug appear in release w/o developers knowing
3) References:  bug 1223037, bug 1220208, bug 1251256

	arni2033 Reporter
	Comment 1 • 8 years ago

This video was recorded on HiDPI (DPI = 125%). But even with  DPI = 100%  it is almost the same.
Screencast:
> https://dl.dropboxusercontent.com/s/nwor6gfugui0di6/bug%201261049%20comment%201.webm?dl=0

	arni2033 Reporter
	Comment 2 • 8 years ago

Oh, and also, if you ever planned to use Fitt's law to Clear button, then now is the best moment.

Applying Fitt's law to Clear button would mean that it's placed right next to the left border of devtools (so if Firefox window is maximized, then clicking on the leftmost pixel of toolbar will click Clear button). Fitt's law is already used for left scrollbutton in breadcrumbs.

	Brian Grinstead [:bgrins]
	Comment 3 • 8 years ago

I'd suggest we stick with the positioning that's in the console (which also seems to be same as in the netmonitor).  That fits with the general position for buttons with .devtools-toolbar.  Changing that convention so it instead lines up on the left border is a separate discussion.

	Jarda Snajdr [:jsnajdr] Assignee
	Comment 4 • 8 years ago

This patch fixes the button position issues. There are 3 possible causes:

1. Different margins for toolbarbuttons inside a devtools-toolbarbutton-group and outside. Happens in the Performance tool. Fixed by removing the custom styling for devtools-toolbarbutton-group and using margin: 2px 1px for all devtools-toolbarbutton's (in toolbars.css).

2. Different margins for HTML devtools-button and XUL devtools-toolbarbutton. Happens in the Memory tool. Fixed by defining the same margin and padding for both (in toolbars.css) and removing the extra styling from memory.css.

3. Redefined padding-left for a particular toolbar. Happens (potentially, if it had a clear icon) in Inspector. Fixed by removing the extra styling from inspector.css.

I also fixed the styles for buttons' :-moz-focusring so that the outline looks reasonably when focused.

Because of the style unification for toolbar-button-group, the default margin for a toolbar button changed from "2px 3px" to "2px 1px". The spacing is still wide enough and the toolbars look more compact. However, I had to verify that this doesn't break anything. That required changes in animation inspector and responsive.html:

- animation inspector has a customized toolbar style: more compact, with vertical separators between buttons. I fixed the margin and padding styles in animationinspector.css, because the layout would be broken otherwise. Also, removed a devtools-toolbarbutton class from one of the labels, because it makes no sense there.
- responsive.html also redefines the devtools-button appearance almost completely, added a padding definition so that it remains the same as before this patch.

Requesting review from bgrins, please assign to someone else if there's a better person for this.

	Brian Grinstead [:bgrins]
	Comment 5 • 8 years ago

Comment on attachment 8739410 [details] [diff] [review]
Clear button position isn't consistent among tabs

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

Forwarding to Tim (if available)

	Tim Nguyen :ntim
	Comment 6 • 8 years ago

Comment on attachment 8739410 [details] [diff] [review]
Clear button position isn't consistent among tabs

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

r=me assuming you've tested this across platforms.

	Tim Nguyen :ntim
	Comment 7 • 8 years ago

Comment on attachment 8739410 [details] [diff] [review]
Clear button position isn't consistent among tabs

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

::: devtools/client/themes/memory.css
@@ -119,5 @@
> - * Due to toolbar styles of `.devtools-toolbarbutton:not([label])` which overrides
> - * .devtools-toolbarbutton's min-width of 78px, reset the min-width.
> - */
> -#import-snapshot,
> -#clear-snapshots {

Just tested this on OSX, and I think we should keep this rule for #import-snapshot, because otherwise the import button looks tiny.

	Jarda Snajdr [:jsnajdr] Assignee
	Comment 8 • 8 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=52cc0682828f

	Jarda Snajdr [:jsnajdr] Assignee
	Comment 9 • 8 years ago

- fixed the #import-snapshot button style in Memory Tool: turned out that only flex-grow: 1 property is needed.
- did a try run and used the Linux and Windows builds to verify platforms other than OS X

	Pulsebot
	Comment 10 • 8 years ago

https://hg.mozilla.org/integration/fx-team/rev/cad934baac3c

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 11 • 8 years ago
bugherder

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

	Nazir Ahmed Sabbir [:NaSb]
	Comment 12 • 8 years ago

Successfully reproduce this bug on Nightly 48.0a1 (2016-03-31) (Build ID: 20160331030231) on Linux, 64 Bit by following the comment 0's instruction !

This Bug is now verified as fixed on Latest Firefox Nightly (2016-04-14) 

Build ID: 20160414030247
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:48.0) Gecko/20100101 Firefox/48.0
OS: Linux 3.19.0-58-generic x86-64

	arni2033 Reporter
	Comment 13 • 8 years ago

Clear button still twitches up and down when I switch between console and netmonitor. Is that according to new design? Should I ignore that twitching or file a separate bug regarding netmonitor?

	Jarda Snajdr [:jsnajdr] Assignee
	Comment 14 • 8 years ago

Hello Arni, I cannot reproduce the up&down twitching you're describing. I'm on OS X. Could you post a video again, and details about your platform?

However, I found that the icon is not aligned in the Memory tool - it's 1px left relative to the other tools. So I'll have to have another look at this bug anyway.

	Tim Nguyen :ntim
	Comment 15 • 8 years ago

(In reply to arni2033 from comment #13)
> Clear button still twitches up and down when I switch between console and
> netmonitor. Is that according to new design? Should I ignore that twitching
> or file a separate bug regarding netmonitor?

I'm not noticing the clear button moving, but maybe it's content next to it that is making it look like it's moving? or maybe it's specific to your platform (I'm on OSX) ?

(In reply to Jarda Snajdr [:jsnajdr] from comment #14)
> However, I found that the icon is not aligned in the Memory tool - it's 1px
> left relative to the other tools. So I'll have to have another look at this
> bug anyway.
Filed bug 1267433