1183619 - Implement the refreshed design for the 'Add a contact' form

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Mike de Boer [:mikedeboer]]

As the acceptance criteria in bug 1179210 states:

 - "Add a contact" form visual refresh

For the visual design spec, please check out the ones attached to bug 1179210.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

We just talked to Sevaan in IRC, he suggested ignoring the small "dropdown" arrow in the "email" field for now.

Comment 2

[Sevaan Franks [:sevaan]]

Right. No support for multiple emails at this stage.

Comment 3

[Victoria Gerchinhoren [:vicky]]

(In reply to Dan Mosedale (:dmose) - needinfo? me for response from comment #1)
> We just talked to Sevaan in IRC, he suggested ignoring the small "dropdown"
> arrow in the "email" field for now.

Thanks, updated in the mocks ;)

Comment 4

[David Critchley (:dcritch)]

I'm looking for where the border and placeholder colors are specified in the Common Elements document. When I use a color picker on those elements, I get the color C3C3C3, however, I don't see this color specified in the color palette. And this color differs from the mockup border/placeholder colors. I just want to verify that this color is correct. Thanks :)

Comment 5

[David Critchley (:dcritch)]

Attachment: Refresh Hello Add Contact Form

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8641938 [details] [diff] [review]
Refresh Hello Add Contact Form

Marking as obsolete; Dave will be updating this patch some and then submitting for review.

Comment 7

[Victoria Gerchinhoren [:vicky]]

(In reply to David Critchley from comment #4)
> I'm looking for where the border and placeholder colors are specified in the
> Common Elements document. When I use a color picker on those elements, I get
> the color C3C3C3, however, I don't see this color specified in the color
> palette. And this color differs from the mockup border/placeholder colors. I
> just want to verify that this color is correct. Thanks :)

please find the specs here: https://www.dropbox.com/sh/t1sqeqsnxcc0ze7/AACHXziG0s4HJ7c5gC_euCwEa?dl=0

Thanks!

Comment 8

[David Critchley (:dcritch)]

Attachment: Screen Shot 2015-08-04 at 9.58.53 AM.png Displays initial Add Contact screen

Comment 9

[David Critchley (:dcritch)]

Comment on attachment 8643161 [details]
Screen Shot 2015-08-04 at 9.58.53 AM.png
Displays initial Add Contact screen

Carrying forward r+ from Sevaan over vidyo. ui-r+ from Sevaan for interactions. Requesting visual design review ui-r+ from Vicky

Comment 10

[David Critchley (:dcritch)]

Attachment: Screen Shot 2015-08-04 at 9.59.40 AM.png - Showing border for active input box, Active text in input box

Comment 11

[David Critchley (:dcritch)]

Comment on attachment 8643193 [details]
Screen Shot 2015-08-04 at 9.59.40 AM.png - Showing border for active input box, Active text in input box

Requesting visual design review ui-r+ from Vicky

Comment 12

[David Critchley (:dcritch)]

Attachment: Screen Shot 2015-08-04 at 10.11.40 AM.png - Showing red border around invalid entries and hover over state for Add Contact button

Requesting visual design review ui-r+ from Vicky

Comment 13

[David Critchley (:dcritch)]

Attachment: Screen Shot 2015-08-04 at 10.24.51 AM.png - Showing hover state for Cancel button

Requesting visual design review ui-r+ from Vicky

Comment 14

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8643161 [details]
Screen Shot 2015-08-04 at 9.58.53 AM.png
Displays initial Add Contact screen

Panel background should be #FBFBFB (not white)
Fields ok.
"Add contact" button should be #00A9DC and no stroke.
Spacing between the 2 buttons is 10px.

Comment 15

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8643193 [details]
Screen Shot 2015-08-04 at 9.59.40 AM.png - Showing border for active input box, Active text in input box

Active input stroke > OK, Text inside > OK

Incorporate feedback from previous review:
Panel background should be #FBFBFB (not white)
"Add contact" button should be #00A9DC and no stroke.
Spacing between the 2 buttons is 10px.

Comment 16

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8643196 [details]
Screen Shot 2015-08-04 at 10.11.40 AM.png - Showing red border around invalid entries and hover over state for Add Contact button

error input stroke > OK

Incorporate feedback from previous review:
Panel background should be #FBFBFB (not white)
"Add contact" button should be #00A9DC and no stroke.
Spacing between the 2 buttons is 10px.

Comment 17

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8643198 [details]
Screen Shot 2015-08-04 at 10.24.51 AM.png - Showing hover state for Cancel button

hover state > OK

R+ but Incorporate feedback from previous review:
Panel background should be #FBFBFB (not white)
"Add contact" button should be #00A9DC and no stroke.
Spacing between the 2 buttons is 10px.

Comment 18

[David Critchley (:dcritch)]

Attachment: Refresh Hello Add Contact Form

Comment 19

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8643958 [details] [diff] [review]
Refresh Hello Add Contact Form

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

This looks great!  Just a few tweaks needed here, and then we'll be set from a code standpoint.  r=dmose with the attached tweaks.

::: browser/components/loop/content/css/contacts.css
@@ +3,5 @@
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +html {
> +  font-size: 10px;
> +  font-family: sans-serif;

Please add an XXX comment here that this will be changed to a system font in bug 1191398.

@@ +256,5 @@
> +  color: #4a4a4a;
> +}
> +
> +.contact-form .form-content-container {
> +  height: 24.1rem;

Add a comment explaining why this height is necessary, and that it could probably go away if we switched this pane to flexbox model.

@@ +258,5 @@
> +
> +.contact-form .form-content-container {
> +  height: 24.1rem;
> +  margin-top: 4px; /* Based on spacing in Mockup */
> +  overflow-y: auto;

Please switch margin-top to padding-top and remove the overflow-y thing, while linking to http://stackoverflow.com/questions/6204670/css-clean-solution-to-the-margin-collapse-issue-when-floating-an-element

::: browser/components/loop/content/css/panel.css
@@ +192,5 @@
>    width: 100%;
>    outline: none;
> +  border-radius: 4px;
> +  margin: 10px 0;
> +  border: 0.1rem solid #c3c3c3;

Please move this back to px.

@@ +208,5 @@
> +  color: #999;
> +}
> +.content-area input:-ms-input-placeholder {
> +  color: #999;
> +}

THis code is always running in Firefox, so no need for the other prefixes.

@@ +425,5 @@
>  .button-group {
>    display: flex;
>    flex-direction: row;
>    width: 100%;
> +  padding-top:6px;

Nit: please fix the whitespace.

@@ +472,4 @@
>    color: #fff;
>  }
>  
>  .button.button-accept:hover {

Please group these together or say why not as well.

::: browser/components/loop/ui/ui-showcase.jsx
@@ +794,5 @@
> +                           roomStore={roomStore}
> +                           selectedTab="contacts_add"
> +                           userProfile={{email: "[e-mail]"}} />
> +                </div>
> +            </FramedExample>

This wants to live in the PanelViewSection.  :-)

Comment 20

[David Critchley (:dcritch)]

Attachment: Screen Shot 2015-08-04 at 9.58.53 AM.png - Initial add contact form. Requested changes completed

Panel background now #FBFBFB
"Add contact" button background is #00A9DC with no stroke.
Space between the Cancel and Add buttons is 10px

Comment 21

[David Critchley (:dcritch)]

Attachment: Screen Shot 2015-08-05 at 4.30.43 PM.png - Show active input box

Added for verification that all review fixes were addressed

Comment 22

[David Critchley (:dcritch)]

Attachment: Screen Shot 2015-08-05 at 4.34.03 PM.png - Shows invalid text input boxes and hover over cancel button

For verification that all review changes have been made

Comment 23

[David Critchley (:dcritch)]

Attachment: Screen Shot 2015-08-05 at 4.34.16 PM.png - hover over Add Contact button

Added for verification that review changes are done

Comment 24

[David Critchley (:dcritch)]

(In reply to Dan Mosedale (:dmose) - needinfo? me for response from comment #19)
> Comment on attachment 8643958 [details] [diff] [review]
> Refresh Hello Add Contact Form
> 
> Review of attachment 8643958 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks great!  Just a few tweaks needed here, and then we'll be set from
> a code standpoint.  r=dmose with the attached tweaks.
> 
> ::: browser/components/loop/content/css/contacts.css
> @@ +3,5 @@
> >   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> >  
> > +html {
> > +  font-size: 10px;
> > +  font-family: sans-serif;
> 
> Please add an XXX comment here that this will be changed to a system font in
> bug 1191398.
> 
> @@ +256,5 @@
> > +  color: #4a4a4a;
> > +}
> > +
> > +.contact-form .form-content-container {
> > +  height: 24.1rem;
> 
> Add a comment explaining why this height is necessary, and that it could
> probably go away if we switched this pane to flexbox model.
> 
> @@ +258,5 @@
> > +
> > +.contact-form .form-content-container {
> > +  height: 24.1rem;
> > +  margin-top: 4px; /* Based on spacing in Mockup */
> > +  overflow-y: auto;
> 
> Please switch margin-top to padding-top and remove the overflow-y thing,
> while linking to
> http://stackoverflow.com/questions/6204670/css-clean-solution-to-the-margin-
> collapse-issue-when-floating-an-element
> 
> ::: browser/components/loop/content/css/panel.css
> @@ +192,5 @@
> >    width: 100%;
> >    outline: none;
> > +  border-radius: 4px;
> > +  margin: 10px 0;
> > +  border: 0.1rem solid #c3c3c3;
> 
> Please move this back to px.
> 
> @@ +208,5 @@
> > +  color: #999;
> > +}
> > +.content-area input:-ms-input-placeholder {
> > +  color: #999;
> > +}
> 
> THis code is always running in Firefox, so no need for the other prefixes.
> 
> @@ +425,5 @@
> >  .button-group {
> >    display: flex;
> >    flex-direction: row;
> >    width: 100%;
> > +  padding-top:6px;
> 
> Nit: please fix the whitespace.
> 
> @@ +472,4 @@
> >    color: #fff;
> >  }
> >  
> >  .button.button-accept:hover {
> 
> Please group these together or say why not as well.
> 
> ::: browser/components/loop/ui/ui-showcase.jsx
> @@ +794,5 @@
> > +                           roomStore={roomStore}
> > +                           selectedTab="contacts_add"
> > +                           userProfile={{email: "[e-mail]"}} />
> > +                </div>
> > +            </FramedExample>
> 
> This wants to live in the PanelViewSection.  :-)

Changes done, squashed like a bug

Comment 25

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8644133 [details]
Screen Shot 2015-08-05 at 4.30.43 PM.png - Show active input box

Great, thanks!

Comment 26

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8644136 [details]
Screen Shot 2015-08-05 at 4.34.16 PM.png - hover over Add Contact button

Thanks!

Comment 27

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8644131 [details]
Screen Shot 2015-08-04 at 9.58.53 AM.png - Initial add contact form. Requested changes completed

Hmmm, not sure about this, I think you attached an old screenshot, right? This has a white background and the button is still with the wrong color and stroke--

Comment 28

[David Critchley (:dcritch)]

Attachment: Screen Shot 2015-08-05 at 4.30.30 PM.png - Updated Initial Contact form screenshot.

Previous attachment was incorrect. This is the correct snapshot of the updated add contact form

Comment 29

[David Critchley (:dcritch)]

Attachment: Screen Shot 2015-08-06 at 3.32.21 PM.png - Fixed color profile so screenshots show correct color now

Comment 30

[David Critchley (:dcritch)]

Attachment: Refresh Hello Add Contact Form

Comment 31

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: add a new contact form visual refresh, ui-r=vicky

New iteration with a bit of CSS and ui-showcase cleanup, ready-to-land.

Comment 32

[Pulsebot]

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

Comment 33

[Victoria Gerchinhoren [:vicky]]

(In reply to Dan Mosedale (:dmose) - needinfo? me for response from comment #31)
> Created attachment 8645925 [details] [diff] [review]
> add a new contact form visual refresh, ui-r=vicky
> 
> New iteration with a bit of CSS and ui-showcase cleanup, ready-to-land.

Can you paste a screenshot for UI review? THanks!

Comment 34

[Ryan VanderMeulen [:RyanVM]]

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

Comment 35

[Dan Mosedale (:dmosedale, :dmose)]

Vicky, it turns out there was an issue there that required some non-trivial work to fix.  I've spun this off to bug 1193569,

Comment 36

[Dan Mosedale (:dmosedale, :dmose)]

and we'll post a patch/screenshot for ui-review there.