feat(devices): add mobile get app placeholders #4261
Conversation
|
Just the minor nit, maybe better for another PR? Otherwise, LGTM |
| @@ -23,8 +23,11 @@ define(function (require, exports, module) { | |||
| var UTM_PARAMS = '?utm_source=accounts.firefox.com&utm_medium=referral&utm_campaign=fxa-devices'; | |||
| var DEVICES_SUPPORT_URL = 'https://support.mozilla.org/kb/fxa-managing-devices' + UTM_PARAMS; | |||
| var FIREFOX_DOWNLOAD_LINK = 'https://www.mozilla.org/firefox/new/' + UTM_PARAMS; | |||
| var FIREFOX_ANDROID_DOWNLOAD_LINK = 'https://www.mozilla.org/firefox/android/' + UTM_PARAMS; | |||
| var FIREFOX_IOS_DOWNLOAD_LINK = 'https://www.mozilla.org/firefox/ios/' + UTM_PARAMS; | |||
| var FIREFOX_ANDROID_DOWNLOAD_LINK = 'https://app.adjust.com/2uo1qc' + | |||
vbudhram
Oct 11, 2016
Contributor
Should we pull all these into constants file? Looks like it is done both way in view files.
Should we pull all these into constants file? Looks like it is done both way in view files.
shane-tomlinson
Oct 12, 2016
Member
If the URLs are shared perhaps, otherwise seems fine to leave them here. Might as well const these things while you are at it!
If the URLs are shared perhaps, otherwise seems fine to leave them here. Might as well const these things while you are at it!
vladikoff
Oct 24, 2016
Author
Contributor
Yeah urls are different, it has a different campaign.
Yeah urls are different, it has a different campaign.
|
A couple of tiny changes requested, otherwise the code looks great! I have not yet manually tested this. |
| <li class="client mobile"> | ||
| <div class="client-name">{{#t}}Firefox for iOS{{/t}}</div> | ||
| <div class="last-connected">none connected</div> | ||
| <a target="_blank" href="{{ linkIOS }}" class="settings-button secondary" data-get-app="ios">{{#t}}Get app{{/t}}</a> |
shane-tomlinson
Oct 12, 2016
Member
Can settings ever be viewed from within the Fx for iOS WebView where plinks are converted to their visible text](
)?
Can settings ever be viewed from within the Fx for iOS WebView where plinks are converted to their visible text](
)?
vladikoff
Oct 24, 2016
Author
Contributor
Yea need to kill these on mobile and only show on desktop.
Yea need to kill these on mobile and only show on desktop.
| @@ -23,8 +23,11 @@ define(function (require, exports, module) { | |||
| var UTM_PARAMS = '?utm_source=accounts.firefox.com&utm_medium=referral&utm_campaign=fxa-devices'; | |||
| var DEVICES_SUPPORT_URL = 'https://support.mozilla.org/kb/fxa-managing-devices' + UTM_PARAMS; | |||
| var FIREFOX_DOWNLOAD_LINK = 'https://www.mozilla.org/firefox/new/' + UTM_PARAMS; | |||
| var FIREFOX_ANDROID_DOWNLOAD_LINK = 'https://www.mozilla.org/firefox/android/' + UTM_PARAMS; | |||
| var FIREFOX_IOS_DOWNLOAD_LINK = 'https://www.mozilla.org/firefox/ios/' + UTM_PARAMS; | |||
| var FIREFOX_ANDROID_DOWNLOAD_LINK = 'https://app.adjust.com/2uo1qc' + | |||
shane-tomlinson
Oct 12, 2016
Member
If the URLs are shared perhaps, otherwise seems fine to leave them here. Might as well const these things while you are at it!
If the URLs are shared perhaps, otherwise seems fine to leave them here. Might as well const these things while you are at it!
| * @private | ||
| */ | ||
| _hasMobileDevices (clients) { | ||
| return _.find(clients, function (client) { |
| /** | ||
| * Returns true if the clients collection has mobile devices | ||
| * | ||
| * @param {Array} clients - array of attached clients |
|
|
||
| return numberOfDevices === 1 ? true : null; | ||
| }, [ TEST_DEVICE_NAME_UPDATED ], 10000)) | ||
|
|
||
| // clicking disconnect on the current device should sign you out | ||
| .findByCssSelector('.client:nth-child(1) .client-disconnect') | ||
| .findByCssSelector('.client-device:nth-child(1) .client-disconnect') |
shane-tomlinson
Oct 12, 2016
Member
You should be able to convert this and the next to lines to use .then(click(...
You should be able to convert this and the next to lines to use .then(click(...
Added to match proposed design in: https://github.com/mozilla/fxa/tree/master/features/FxA-89-devices-view#zero-or-one-device-views
@shane-tomlinson @vbudhram r?