869131 - [Email][Activity Indicator] Wrong use of spinner when setting up account

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect, P1)

Description

[Eric Pang [:epang] UX]

Attachment: Setting up email account screen

Adding an email account screen needs to be updated to match building blocks.

Building blocks can be found here (follow the three activity and progress modal with the spinner (also see attached mock up/spec):

https://wiki.mozilla.org/Gaia/Design/BuildingBlocks#Progress_.26_Activity_Indicators

Type is 16px 400 weight (regular)

Comment 1

[Eric Pang [:epang] UX]

(In reply to Eric Pang [:epang] from comment #0)
> Created attachment 746016 [details]
> Setting up email account screen
> 
> Adding an email account screen needs to be updated to match building blocks.
> 
> Building blocks can be found here (follow the three activity and progress
> modal with the spinner (also see attached mock up/spec):
> 
> https://wiki.mozilla.org/Gaia/Design/BuildingBlocks#Progress_.
> 26_Activity_Indicators
> 
> Type is 16px 400 weight (regular)

Also, Victoria, can you confirm the cancel button should be added? Thanks!

Comment 2

[Victoria Gerchinhoren [:vicky]]

Hi Eric,
If devs just grab the building block, I understand there's no need to specify spaces and font sizes, since everything should be in the building block overlay code. 

The Cancel button should go in every overlay since the user needs to have a "way out".

Thanks!

Comment 3

[Eric Pang [:epang] UX]

(In reply to Victoria Gerchinhoren from comment #2)
> Hi Eric,
> If devs just grab the building block, I understand there's no need to
> specify spaces and font sizes, since everything should be in the building
> block overlay code. 
> 
> The Cancel button should go in every overlay since the user needs to have a
> "way out".
> 
> Thanks!

Great, thanks for letting me know!

Comment 4

[AxelR]

Hi Victoria, 

If it's possible, I would like to work on this bug. Could I be assigned on this bug ?

Thanks

Comment 5

[Eric Pang [:epang] UX]

(In reply to AxelR from comment #4)
> Hi Victoria, 
> 
> If it's possible, I would like to work on this bug. Could I be assigned on
> this bug ?
> 
> Thanks

Would be great if you can!  Victoria I've assigned the bug to  AxelR, is this okay? Thanks!

Comment 6

[Victoria Gerchinhoren [:vicky]]

Be my guest ;)

Comment 7

[AxelR]

Attachment: pull request

Comment 8

[AxelR]

I think it's fixed.
Here is the pull request : https://github.com/mozilla-b2g/gaia/pull/10277

Comment 9

[Eric Pang [:epang] UX]

(In reply to AxelR from comment #8)
> I think it's fixed.
> Here is the pull request : https://github.com/mozilla-b2g/gaia/pull/10277

Hi, can you add a screen shot of the update so I can check the visuals? Thanks!

Comment 10

[AxelR]

Attachment: patch screenshot

Hi Eric, please find attached a screenshot of my patch

Comment 11

[Kevin Grandon :kgrandon]

Comment on attachment 760149 [details] [review]
pull request

I'm sorry - I dropped the ball on this one. We'll need to rebase this, and please mark :asuth as a reviewer. Thanks!

Comment 12

[Eric Pang [:epang] UX]

(In reply to Kevin Grandon :kgrandon from comment #11)
> Comment on attachment 760149 [details] [review]
> pull request
> 
> I'm sorry - I dropped the ball on this one. We'll need to rebase this, and
> please mark :asuth as a reviewer. Thanks!

Hey Kevin, i did too!  The latest (as in 2 months ago looked good).  Flagged AxelR for needinfo to make sure your comment is seen.  Thanks!

Comment 13

[AxelR]

Hey all, 

No problem I will rebase it asap (this week-end it should be ok)

Comment 14

[AxelR]

Attachment: rebased patch

Rebased patch

Comment 15

[Andrew Sutherland [:asuth] (he/him)]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/14008

Pointer to Github pull-request

Comment 16

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8337670 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/14008

There was some bit-rot due to this having fallen through the cracks, specifically the l10n id changed.  I fixed this and made some other minor review cleanup fixes.  Because transition guidelines also call for this dialog style to fade in rather than slide in I have made preparatory changes so that when "anim-fade" support lands as part of bug 940132, the card will just fade in.  For now, it slides in, which is dumb, but bug 940132 is a definite 1.3 thing, so I'm not too worried.

I'll land this once I get a green travis run.  The tree/tests seem slightly messed up right now, so I'll probably spend some time clicking random buttons until that happens.

Comment 17

[Andrew Sutherland [:asuth] (he/him)]

landed on gaia/master:
https://github.com/mozilla-b2g/gaia/pull/14008
https://github.com/mozilla-b2g/gaia/commit/9935ceb4fe96fff48e78b941cfd2a69d8639a418

As noted above, the transition will get pretty when bug 940132 lands.

Comment 18

[Andrew Sutherland [:asuth] (he/him)]

And of course, thanks again for the patch AxelR and making the effort to perform the initial rebase!  And my apologies about the unreasonably long time for the review.