-
Notifications
You must be signed in to change notification settings - Fork 221
feat: add new strapi customization for user expectation on the final authentication screens #19959
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
Conversation
|
PR for strapi: mozilla/fxa-strapi#161 |
packages/fxa-settings/src/pages/Signin/SigninTokenCode/index.stories.tsx
Show resolved
Hide resolved
| </AppContext.Provider> | ||
| ); | ||
|
|
||
| export const OAuthDesktopServiceRelay = () => ( |
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.
Same comment as before, for all stories - we should make sure that all pages have a story with the additional accessibility info so we can ensure the styling looks as expected.
| recoveryKeyExists, | ||
| integrationIsSync, | ||
| isFirefoxClientServiceRelay, | ||
| additionalAccessibilityInfo, |
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 page differs from the others in passing the text as a prop instead of retrieving cmsInfo in index.tsx
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 page doesn't have integration passed in, but yeah I probably should keep it consistent
0d3f37a to
ae7ed86
Compare
…authentication screens Because: * we want to customize "Firefox will try sending you back to use an email mask after you sign in." because we need something similar for smart window This commit: * adds additionalAccessibilityInfo (name suggested by Ross) to cmsInfo.shared Closes FXA-12638
ae7ed86 to
533a110
Compare
vpomerleau
left a comment
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.
Thank you for the updates! I tested locally and this is working as designed.
We should make sure to merge fxa-strapi stg into prod as soon as possible and add the known customization for the relay entrypoint so it can be localized before train 329/330 prod rollout.
entrypoint=relay_integration
client_id=5882386c6d801776
A couple of considerations for future improvements (to discuss with Ross/Zach):
- I initially thought the customization wasn't pulled in, but eventually realized that the custom text is not included on either the index page or the signup page. Would be nice to add there for consistency.
- I actually prefer the message in info banner as you had accidentally copy-pasted, as long as the banner is placed in the usual location (below heading), compared to the current text version, especially when there are two sentences like so:
Because
This pull request
Issue that this pull request solves
Closes: FXA-12638
Checklist
Put an
xin the boxes that applyScreenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
Any other information that is important to this pull request.