-
Notifications
You must be signed in to change notification settings - Fork 210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(pair): Create various UX for pair/unsupported #17517
Conversation
@@ -1,12 +1,9 @@ | |||
<div class="card-base"> | |||
<header> | |||
{{#isDesktopNonFirefox}} | |||
{{#showCADHeader}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This conditional was easier than juggling it here in the mustache file.
packages/fxa-content-server/app/scripts/templates/pair/unsupported.mustache
Outdated
Show resolved
Hide resolved
packages/fxa-content-server/app/scripts/templates/pair/failure.mustache
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LZoog I didn't test all of the states but overall seems to work well. Lmk your thoughts on the questions.
</h1> | ||
</header> | ||
|
||
<p class="mt-3">{{#t}}The setup couldn’t be completed.{{/t}}{{#showSigninLink}} {{#t}}Please sign in with your email.{{/t}}{{/showSigninLink}}</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QA recently reopened https://mozilla-hub.atlassian.net/browse/FXA-10258 and I have been stratching my head on this. I think the best thing to do is to remove the button completely and tell the user to login with their email.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also be able to close the above ticket as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm was that a requested feature? If we shouldn't spend more time trying to fix it (I'm also not sure what's going on without investigating, I see the params are passed), I'm OK with removing it and revisiting when we do React work on that page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chatted in Slack and Vijay will remove this with the linked ticket in a quick follow up after this lands.
packages/fxa-content-server/app/scripts/views/pair/unsupported.js
Outdated
Show resolved
Hide resolved
Because: * This page is confusing and not ideal for our users and we want to render various new UX to help them complete the pair flow This commit: * Adjusts 'Pair using an app' screen according to design, only show users this when the URL reflects a state that the user tried pairing with their system camera * Adds a new 'Connecting your mobile device' state for when mobile users (not through system camera) reach this page, and conditionally renders an alert with 'Download Firefox now' link for iOS and Android if user is not on FF * Previously, the 'default state' for this page was the system camera screen. This commit adds a new generic error state * Tweaks pair/failure to match the new style/image, hides "Sign in with email" text if button is not shown * Adds automatic Glean click events cad_redirect_mobile_download and cad_redirect_mobile_learn_more, adds custom events cad_redirect_mobile_view and cad_redirect_desktop_default_view closes FXA-10136
Going to go ahead and merge, test is an unrelated MySQL timeout. |
Because:
This commit:
closes FXA-10136
--
These screenshots have been sent to Eduardo and are approved from what he saw. You can see these by changing the UA in dev tools.
Edit: these are mostly up to date, but now I've added a class so
firefox.com/pair
is always on the same line.bonus
pair/failure
updates: