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

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).

Comment on attachment 8712832 [details] Screenshot Added screenshot + notes above!

Another note (sorry): Background should be var ----theme-body-background.

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

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.

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.

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.

(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).

(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.