Gabriel [:gl] (ΦωΦ) Assignee
	Description • 9 years ago

We should implement the basic design of the responsive design mode whether that means centering it, adding the appropriate background colour, etc.

	Helen V. Holmes (:helenvholmes) (:✨)
	Comment 3 • 9 years ago

Added a screenshot with general notes, repeated here:

- Border on the viewport should be: border: 1px solid #DDE1E4;
- Text-shadow on the viewport: text-shadow: 0 4px 4px 0  rgba(155, 155, 155, 0.26);
- The top area is ~16px tall.
- background of the top area is #fcfcfc;
- the color in the top area is #696969;
- the border of the top area is the same as the rest of the viewport.

Also:

Corners on the viewport are /not/ rounded.
Corners on the top menu area /are/ rounded. (Border-radius 1px).

	Helen V. Holmes (:helenvholmes) (:✨)
	Comment 4 • 9 years ago

Comment on attachment 8712832 [details]
Screenshot

Added screenshot + notes above!

	Helen V. Holmes (:helenvholmes) (:✨)
	Comment 5 • 9 years ago

Another note (sorry):

Background should be var ----theme-body-background.

	Gabriel [:gl] (ΦωΦ) Assignee
	Comment 8 • 9 years ago

I think one of the things we need to think about is how it should look like on the dark theme.

	Tim Nguyen :ntim
	Comment 9 • 9 years ago

Comment on attachment 8712850 [details] [diff] [review]
1243403.patch [1.0]

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

I heavily think we should avoid using hardcoded colors, especially if we want the responsive mode shell to adapt to themes.

Bug 1242709 makes our theme colors much closer to the ones used in this patch.

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)
	Comment 11 • 9 years ago

Comment on attachment 8712887 [details] [diff] [review]
1243403.patch [2.0]

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

r+ assuming ntim agrees, he knows the theme colors much better than me.

	Tim Nguyen :ntim
	Comment 12 • 9 years ago

Comment on attachment 8712887 [details] [diff] [review]
1243403.patch [2.0]

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

Minor comment about CSS organisation:
I think we should start adding headers like on the memory tool CSS [0] to clearly separate each section of the CSS, like this:
/**
 * Viewport container
 */
This will help us keep our CSS clean, especially when more rules are added.

[0]: https://dxr.mozilla.org/mozilla-central/source/devtools/client/themes/memory.css#134

::: devtools/client/responsive.html/index.css
@@ +26,1 @@
>  iframe {

I would prefix this selector with .viewport to explicit what we're actually styling here.

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)
	Comment 13 • 9 years ago

(In reply to Tim Nguyen [:ntim] from comment #12)
> ::: devtools/client/responsive.html/index.css
> @@ +26,1 @@
> >  iframe {
> 
> I would prefix this selector with .viewport to explicit what we're actually
> styling here.

Good idea, or can also replace with ".browser" (which is on the iframe).

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)
	Comment 14 • 9 years ago

(In reply to Tim Nguyen [:ntim] from comment #12)
> Comment on attachment 8712887 [details] [diff] [review]
> 1243403.patch [2.0]
> 
> Review of attachment 8712887 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Minor comment about CSS organisation:
> I think we should start adding headers like on the memory tool CSS [0] to
> clearly separate each section of the CSS, like this:
> /**
>  * Viewport container
>  */
> This will help us keep our CSS clean, especially when more rules are added.

Yes, it seems fine for now.  Like the TODO says at the top, I think we may switch to CSS file per component once we have utility to load them easily.  But for now, comments are a good way.

	Pulsebot
	Comment 16 • 9 years ago

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

	Carsten Book [:Tomcat]
	Comment 17 • 9 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/1c2c044db3ef