Skip to content

fix(settings): break long email in verification code pages#18832

Merged
MagentaManifold merged 1 commit intomainfrom
FXA-10726
May 8, 2025
Merged

fix(settings): break long email in verification code pages#18832
MagentaManifold merged 1 commit intomainfrom
FXA-10726

Conversation

@MagentaManifold
Copy link
Copy Markdown
Contributor

Because

  • The “Enter your confirmation code” page has UI issues on mobile when using a long email address

This pull request

  • breaks long email into multiple lines so that the lines does not grow out of the box (in both ConfirmSignupCode and SigninTokenCode)

Issue that this pull request solves

Closes: FXA-10726

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).

Screenshots (Optional)

image

@MagentaManifold MagentaManifold requested a review from a team as a code owner May 7, 2025 17:44
@MagentaManifold
Copy link
Copy Markdown
Contributor Author

Unfortunately this only works in Storybook because it only works with the fallback, not localized strings.

@MagentaManifold MagentaManifold marked this pull request as draft May 7, 2025 18:08
@MagentaManifold MagentaManifold force-pushed the FXA-10726 branch 2 times, most recently from a20099e to 60bad4e Compare May 7, 2025 20:42
@MagentaManifold MagentaManifold marked this pull request as ready for review May 7, 2025 20:43
@MagentaManifold MagentaManifold requested a review from a team as a code owner May 7, 2025 20:43
Copy link
Copy Markdown
Contributor

@vpomerleau vpomerleau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, tested locally and works as intended! The only change I would recommend before merging is using <FtlMsg>, we are standardizing towards that custom wrapper.

);
screen.getByText(
`Enter the code that was sent to ${MOCK_EMAIL} within 5 minutes.`
(_, element) =>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice way to account for the <span> 👍

<EmailCodeImage />

<FtlMsg id="signin-token-code-instruction" vars={{ email }}>
<Localized
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<FtlMsg> (custom wrapper) is preferred over <Localized>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PageSecondaryEmailVerify used Localized so I thought it does something different from FtlMsg. TIL FtlMsg is literally just a one-line wrapper to enforce a child prop. Thanks for your suggestions!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of the intent of the wrapper is to unblock more customized localization testing in the future. Pages with "Localized" are older and were created before the wrapper was introduced.

<EmailCodeImage />

<FtlMsg id="confirm-signup-code-instruction" vars={{ email: email! }}>
<Localized
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, <FtlMsg> is preferred

Because:

* The “Enter your confirmation code” page has UI issues on mobile when using a long email address

This commit:

* breaks long email into multiple lines so that the lines does not grow out of the box (in both ConfirmSignupCode and SigninTokenCode)

Closes FXA-10726
@MagentaManifold MagentaManifold merged commit e1b04e9 into main May 8, 2025
19 checks passed
@MagentaManifold MagentaManifold deleted the FXA-10726 branch May 8, 2025 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants