It looks like the recent style changes had an effect on the remote connection screen. Some things are an improvement, like the way the port number input field is rendered, or the rounded borders for input boxes on Linux. What I find most jarring is the white background on input fields and the button, and maybe the lack of rounded borders on Mac.

We need to move the .devtools-textinput styles into common.css (from toolbars.inc.css) so that they are included for the rest of the browser chrome.

Weird, min-height doesn't seem to apply to input[type=number] so just copying over the styles causes mismatched heights. We could remove the type=number and it will look like it used to.

IIRC we were thinking about shrinking the huge input boxes a few months back, so you could just drop min-height.

The only design I see with the textbox in it is this one: https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/DarkTheme-Console-AllToggled@2x.png, which does seem to have a smaller total height (as well as a different background color).

Darrin, I'm looking at theming the text boxes to match the new designs (https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/DarkTheme-Console-AllToggled@2x.png). The textbox seems to have a background, rgb(21,25,30), which is quite close to the sidebar background, rgb(24, 29, 32), as seen here: https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors. Should I set the input background as it is in the image, or should I match it to one of the palette colors from the theme? Also, the text color seems to be something like rgb(119, 122, 123) - should I use this or grab one of the colors from the theme?

(In reply to Panos Astithas [:past] from comment #0) > Created attachment 8345193 [details] > How it looks now > I think this looks better than what we had until now :)

I would say use the (dark) background color from the mockup, and a similar border color (from the mockup) as well. The text color in the mockup seems to be for placeholder text (usually less visible), so I would suggest one of the brighter colors from the palette for the text (maybe rgba(184, 200, 217, 1)?). Did we make any decisions regarding rounded text boxes?

(In reply to Darrin Henein [:darrin] from comment #8) > I would say use the (dark) background color from the mockup, and a similar > border color (from the mockup) as well. The text color in the mockup seems > to be for placeholder text (usually less visible), so I would suggest one of > the brighter colors from the palette for the text (maybe rgba(184, 200, > 217, 1)?). > > Did we make any decisions regarding rounded text boxes? You are needinfo'ed on bug 947346 for that decision only :)

> Did we make any decisions regarding rounded text boxes? There is some disagreement in Bug 947346 about this. By the way, do you know what the border radius for the text boxes is in the mockup?

(In reply to Victor Porof [:vp] from comment #7) > (In reply to Panos Astithas [:past] from comment #0) > > Created attachment 8345193 [details] > > How it looks now > > > > I think this looks better than what we had until now :) Yes, it really doesn't look too bad now (without the styling applied). It isn't consistent with the rest of the tools, but it is kind of a separate page. I'd be fine going either way with this.

The white background color of the text boxes does look out of place to me though.

> By the way, do you know what the border radius for the text boxes is in the > mockup? It looks like 50% (circular) to me. Does that help?

Darrin, Here is a screenshot of the work in progress styling on the text boxes. You can see them both on the connect screen and on the . I've pushed to try so that a build can be played with to get a feel for the different states: https://tbpl.mozilla.org/?tree=Try&rev=61652a666c8f. See anything that should be updated in order to match the designs at https://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/? Some of the states aren't pictured in the designs, so I just picked something (see the red validation error in the inspector in the screenshot, for instance).

> You can see them both on the connect screen and on the . I meant to say you can see the styling on both the connect screen on and on the inspector search box.

The patch going along with the screenshot in Attachment 8346222 [details]. Panos, does this look good (assuming the textbox design passes ui-review)?

Comment on attachment 8346222 [details] Updated text input styles.png This look great.

Comment on attachment 8348764 [details] [diff] [review] theme-connect-screen.patch Review of attachment 8348764 [details] [diff] [review]: ----------------------------------------------------------------- I'm surprised we want to use an unstyled button element, but I'm no UX expert, so OK. Funny detail: a high-contrast white button makes its vertical misalignment more apparent, but I don't really care to be honest. It did look better in attachment 8345193 [details] though.

(In reply to Panos Astithas [:past] from comment #18) > ----------------------------------------------------------------- > > I'm surprised we want to use an unstyled button element, but I'm no UX > expert, so OK. It was my understanding that this bug addresses the styling of the text inputs only. Should we be discussing the buttons as well? I am fine with that, just didn't want to introduce new scope to this bug.

(In reply to Darrin Henein [:darrin] from comment #19) > (In reply to Panos Astithas [:past] from comment #18) > > ----------------------------------------------------------------- > > > > I'm surprised we want to use an unstyled button element, but I'm no UX > > expert, so OK. > > It was my understanding that this bug addresses the styling of the text > inputs only. Should we be discussing the buttons as well? I am fine with > that, just didn't want to introduce new scope to this bug. I guess we may as well go ahead and talk about that button here, as well. In Bug 947836, it was decided to not move the button styles over into the shared common.css file, to prevent copying too much across to common.css (a file which is loaded inside the main browser UI). I just thought of a different solution that could work, though - we could load the dark theme CSS file into the connect screen. This would allow us to get both the button and the new textbox styling without needing to introduce a bunch of global styles to the browser. Will attach a screenshot of this.

Using dark-theme.css for textbox and button styling, also moves the button over a bit to the right to vertically align with the text boxes

Comment on attachment 8348803 [details] connect-screen-with-dark-theme Ya, this is much better. Can we align the button with the right edge of the text inputs though (vs the left as seen)?

Panos, This should do the same thing, but updates the button as well (and right aligns it with the text boxes). Can you think of any reason not to include dark-theme.css in this connect.xhtml file? This also updates the selection text color (for all elements, but the main reason was for readability of selected text for inputs), as discussed with Darrin in IRC.

Updated button alignment, also shows selected text coloring

Comment on attachment 8348841 [details] connect-screen-with-dark-theme Much better, looks awesome. Thanks Brian!

See Comment 23

Comment on attachment 8348846 [details] [diff] [review] theme-connect-screen.patch Review of attachment 8348846 [details] [diff] [review]: ----------------------------------------------------------------- This seems fine to me.

https://hg.mozilla.org/integration/fx-team/rev/c835fcc812b2 https://tbpl.mozilla.org/?tree=Fx-Team&rev=c835fcc812b2

Brian, can you uplift this to Aurora as well?

We worked around the min-height issue on input[type=number] by not using min-height, so this doesn't depend on Bug 951310.