Skip to content
This repository has been archived by the owner. It is now read-only.

WIP feat(client): complete on fennec #4370

Merged
merged 5 commits into from Dec 23, 2016
Merged

Conversation

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Nov 3, 2016

Builds on #4395

Fixes #4372

@rfk
Copy link
Member

@rfk rfk commented Nov 3, 2016

❤️

One question I had when I saw this demo was around PII handling. IIUC this will technically share your email address with our SMS provider, if they wanted to go looking for it in the links we're sending out. If so then we should parallelize and seek data-approval at the same time as we're working on the code here.

(I'm having flashbacks to Gravatar where we had to add a permission box to even send them the hash of your email address)

@shane-tomlinson shane-tomlinson force-pushed the shane-tomlinson/complete-on-fennec branch from b02ac05 to 96bca69 Nov 3, 2016
@shane-tomlinson shane-tomlinson changed the title WIP feat(client): Send SMS WIP feat(client): complete on fennec Nov 3, 2016
@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Nov 3, 2016

The title was misnamed, this is only "open verification link in fennec and allow the user to sign in". SMS is a much larger project that will not be part of this PR.

We are developing this portion independently so that it can merge w/o being blocked on SMS work.

@shane-tomlinson shane-tomlinson force-pushed the shane-tomlinson/complete-on-fennec branch 3 times, most recently from 2dae1dc to 50d4736 Nov 3, 2016

<form novalidate>
<div class="button-row">
<a class="button" href="{{{ escapedSignInUrl }}}">{{#unsafeTranslate}}Continue &rarr;{{/unsafeTranslate}}</a>

This comment has been minimized.

@vladikoff

vladikoff Nov 4, 2016
Contributor

please add a class or an id to this button

@vladikoff vladikoff force-pushed the shane-tomlinson/complete-on-fennec branch 2 times, most recently from b25a462 to 414f408 Nov 4, 2016
@shane-tomlinson shane-tomlinson force-pushed the shane-tomlinson/complete-on-fennec branch 3 times, most recently from e2b90b1 to a6b4318 Nov 6, 2016
assert.equal(resultText, expected);
})
.end();
.then(testAttributeEquals(selector, href, expected));

This comment has been minimized.

@vladikoff

vladikoff Nov 7, 2016
Contributor

👍 niiice

@shane-tomlinson shane-tomlinson force-pushed the shane-tomlinson/complete-on-fennec branch 3 times, most recently from f84ca31 to 5b1319d Nov 8, 2016
@shane-tomlinson shane-tomlinson self-assigned this Nov 11, 2016
@shane-tomlinson shane-tomlinson force-pushed the shane-tomlinson/complete-on-fennec branch from 5b1319d to 048eca7 Nov 11, 2016
@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Nov 14, 2016

#4395 landed so we can now use UAP for this

@shane-tomlinson shane-tomlinson force-pushed the shane-tomlinson/complete-on-fennec branch 2 times, most recently from 426f0fe to 41abd42 Nov 14, 2016
@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Nov 17, 2016

@shane-tomlinson is this waiting for the feature doc phase 1?

@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Nov 17, 2016

@shane-tomlinson is this waiting for the feature doc phase 1?

It's waiting on that, and on time from me. I'm pulling parts out of this to merge independently, will have a PR later.

@shane-tomlinson shane-tomlinson added this to the FxA-51: Email Confirmation Flow (Phase 1) milestone Nov 17, 2016
@shane-tomlinson shane-tomlinson force-pushed the shane-tomlinson/complete-on-fennec branch from 41abd42 to 2a5369a Nov 17, 2016
@shane-tomlinson shane-tomlinson force-pushed the shane-tomlinson/complete-on-fennec branch from ebbe93e to 265a926 Dec 19, 2016
@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Dec 19, 2016

@shane-tomlinson on a 'tablet' resolution I noticed that the modal gets the 100% width:
not sure if we need to change any of that...

What is the resolution? It looks like it's hitting the "@media (max-width: 520px), (orientation: landscape) and (min-width: 481px) and (max-height: 480px) {" media query which is meant for mobile phones in landscape mode, that's how the @include repond-to('small') mixin works.

I'm not sure what the best approach here is. I'm tempted to say "that's fine for mobile." Throwing this PR up on stomlinson.dev.lcip.org to try on my iPad 2 to see how it responds on a real tablet in landscape mode.

shane-tomlinson pushed a commit that referenced this pull request Dec 19, 2016
This is an extraction from #4370 to make both simpler to review. For
that PR, we need to be able to configure which app store logos are
displayed to the user. This allows us to do so.

The marketing-mixin module, instead of returning a mixin, returns
a function that must be called with configuration options, the function
returns the actual mixin.

The marketing_snippet module allows two additional configuration
options:
* `marketingId` is the id used to log impressions and clicks
* `which` forces specific logo(s) to be displayed. If not specified,
uses default behavior.

The Adjust links are extracted into Constants.js and shared between
marketing_snippet.js and settings/clients.js.

base.js->getSearchParam has been added to fetch
a single search parameter.

Add `isFirefoxAndroid`, `isFirefoxIos` and `isFirefoxDesktop` to
user-agent.
@shane-tomlinson shane-tomlinson force-pushed the shane-tomlinson/complete-on-fennec branch from 856ddd4 to 237fe44 Dec 20, 2016
shane-tomlinson pushed a commit that referenced this pull request Dec 20, 2016
This is an extraction from #4370 to make both simpler to review. For
that PR, we need to be able to configure which app store logos are
displayed to the user. This allows us to do so.

The marketing-mixin module, instead of returning a mixin, returns
a function that must be called with configuration options, the function
returns the actual mixin.

The marketing_snippet module allows two additional configuration
options:
* `marketingId` is the id used to log impressions and clicks
* `which` forces specific logo(s) to be displayed. If not specified,
uses default behavior.

The Adjust links are extracted into Constants.js and shared between
marketing_snippet.js and settings/clients.js.

base.js->getSearchParam has been added to fetch
a single search parameter.

Add `isFirefoxAndroid`, `isFirefoxIos` and `isFirefoxDesktop` to
user-agent.
@shane-tomlinson shane-tomlinson force-pushed the shane-tomlinson/complete-on-fennec branch from 237fe44 to a75a880 Dec 20, 2016
shane-tomlinson pushed a commit that referenced this pull request Dec 20, 2016
This is an extraction from #4370 to make both simpler to review. For
that PR, we need to be able to configure which app store logos are
displayed to the user. This allows us to do so.

The marketing-mixin module, instead of returning a mixin, returns
a function that must be called with configuration options, the function
returns the actual mixin.

The marketing_snippet module allows two additional configuration
options:
* `marketingId` is the id used to log impressions and clicks
* `which` forces specific logo(s) to be displayed. If not specified,
uses default behavior.

The Adjust links are extracted into Constants.js and shared between
marketing_snippet.js and settings/clients.js.

base.js->getSearchParam has been added to fetch
a single search parameter.

Add `isFirefoxAndroid`, `isFirefoxIos` and `isFirefoxDesktop` to
user-agent.
shane-tomlinson pushed a commit that referenced this pull request Dec 21, 2016
This is an extraction from #4370 to make both simpler to review. For
that PR, we need to be able to configure which app store logos are
displayed to the user. This allows us to do so.

The marketing-mixin module, instead of returning a mixin, returns
a function that must be called with configuration options, the function
returns the actual mixin.

The marketing_snippet module allows two additional configuration
options:
* `marketingId` is the id used to log impressions and clicks
* `which` forces specific logo(s) to be displayed. If not specified,
uses default behavior.

The Adjust links are extracted into Constants.js and shared between
marketing_snippet.js and settings/clients.js.

base.js->getSearchParam has been added to fetch
a single search parameter.

Add `isFirefoxAndroid`, `isFirefoxIos` and `isFirefoxDesktop` to
user-agent.
shane-tomlinson pushed a commit that referenced this pull request Dec 21, 2016
…#4519) r=@vladikoff

This is an extraction from #4370 to make both simpler to review. For
that PR, we need to be able to configure which app store logos are
displayed to the user. This allows us to do so.

The marketing-mixin module, instead of returning a mixin, returns
a function that must be called with configuration options, the function
returns the actual mixin.

The marketing_snippet module allows two additional configuration
options:
* `marketingId` is the id used to log impressions and clicks
* `which` forces specific logo(s) to be displayed. If not specified,
uses default behavior.

The Adjust links are extracted into Constants.js and shared between
marketing_snippet.js and settings/clients.js.

base.js->getSearchParam has been added to fetch
a single search parameter.

Add `isFirefoxAndroid`, `isFirefoxIos` and `isFirefoxDesktop` to
user-agent.
@shane-tomlinson shane-tomlinson force-pushed the shane-tomlinson/complete-on-fennec branch 4 times, most recently from 483b1fe to b0eb8ad Dec 21, 2016
Shane Tomlinson added 5 commits Nov 2, 2016
Allows users who open the verification link on a different Firefox
to sign in, or if they open the link on the same device or
a non-Firefox, show links to an app store to install.

This is a squash of work done by both @vladikoff and I.

fixes #4372
Only Fennec > 43 supports web channel Sync login.
A user is only eligible to see the "connect another device" screen if
a different user is not already signed in.

Log if users are already signed in.
Users that are signed in see "This device is connected"
Users that are not signed in see "Email verified"
@shane-tomlinson shane-tomlinson force-pushed the shane-tomlinson/complete-on-fennec branch from b0eb8ad to 6b3c4cd Dec 22, 2016
@vladikoff vladikoff merged commit c80d1aa into master Dec 23, 2016
4 checks passed
4 checks passed
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.09%) to 98.514%
Details
@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Dec 23, 2016

Nice! 👍

@shane-tomlinson shane-tomlinson deleted the shane-tomlinson/complete-on-fennec branch Dec 23, 2016
shane-tomlinson pushed a commit that referenced this pull request Jan 6, 2017
…ent.

A lot of content was accidentally deleted in #4370, this brings back the content.
shane-tomlinson pushed a commit that referenced this pull request Jan 6, 2017
…ent.

A lot of content was accidentally deleted in #4370, this brings back the content.
shane-tomlinson pushed a commit that referenced this pull request Jan 6, 2017
…ent.

A lot of content was accidentally deleted in #4370, this brings back the content.
vladikoff added a commit that referenced this pull request Jan 6, 2017
…ent. (#4593) r=vladikoff

A lot of content was accidentally deleted in #4370, this brings back the content.
divyabiyani added a commit to divyabiyani/fxa-content-server that referenced this pull request Jan 6, 2017
…ent. (mozilla#4593) r=vladikoff

A lot of content was accidentally deleted in mozilla#4370, this brings back the content.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants