Skip to content

fix(settings): VPN polish - CMS copy on set-password, repeat PW when Sync isn't decoupled, service_welcome routing#20817

Open
LZoog wants to merge 1 commit into
mainfrom
FXA-14047
Open

fix(settings): VPN polish - CMS copy on set-password, repeat PW when Sync isn't decoupled, service_welcome routing#20817
LZoog wants to merge 1 commit into
mainfrom
FXA-14047

Conversation

@LZoog

@LZoog LZoog commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Because:

  • The CMS text for passwordless OTP users when Sync is not decoupled is not rendered on the set password page
  • The repeat password field is not displayed when Sync is not decoupled
  • Mobile users should not be navigated to the service welcome screen for VPN because the browser automatically closes it

This commit:

  • Renders the CMS text and repeat password fields accordingly
  • Adds a Mobile check and does not navigate those users to the service welcome screen
  • Returns a fixed requiresPasswordForLogin value from signup mock instead of rederiving it

closes FXA-14047
closes FXA-14088

  • 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).
  • I have manually reviewed all AI generated code.

Copilot AI review requested due to automatic review settings July 3, 2026 16:20
@LZoog LZoog requested a review from a team as a code owner July 3, 2026 16:20

Copilot AI left a comment

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.

Pull request overview

This PR polishes password-setting and post-verification behavior for VPN / non-decoupled Sync scenarios by ensuring the correct CMS copy and password confirmation UI are shown, and by adjusting VPN-specific routing after signup confirmation.

Changes:

  • Prefer CMS description copy on the PostVerify Set Password page for all arrival paths, with hardcoded text as fallback.
  • Update Signup password-confirmation logic to depend on whether a password is required for key derivation (requiresPasswordForLogin), not only on Sync.
  • Avoid navigating mobile VPN users to /post_verify/service_welcome to prevent a brief “blip” before the client closes the webview; add tests/stories/mocks updates.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/fxa-settings/src/pages/Signup/mocks.tsx Extends signup mocks to include requiresPasswordForLogin and adjusts OAuth-native mock signature.
packages/fxa-settings/src/pages/Signup/interfaces.ts Extends signup integration types to include requiresPasswordForLogin.
packages/fxa-settings/src/pages/Signup/index.tsx Uses requiresPasswordForLogin(supportsKeysOptionalLogin) to decide whether to show repeat-password confirmation.
packages/fxa-settings/src/pages/Signup/index.test.tsx Updates expectations and adds coverage for keys-optional vs non-optional password confirmation behavior.
packages/fxa-settings/src/pages/Signup/container.test.tsx Updates the integration mock to include requiresPasswordForLogin.
packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/index.tsx Skips service_welcome navigation for mobile VPN to avoid UI “blip”.
packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/index.test.tsx Adds test to ensure no navigation occurs for mobile VPN.
packages/fxa-settings/src/pages/PostVerify/SetPassword/index.tsx Renders CMS description regardless of arrival reason; falls back to reason-specific hardcoded copy.
packages/fxa-settings/src/pages/PostVerify/SetPassword/index.test.tsx Adds assertions for CMS-vs-fallback copy across OTP/passkey paths; refactors CMS test data usage.
packages/fxa-settings/src/pages/PostVerify/SetPassword/index.stories.tsx Adds OTP-with-CMS story variant.
packages/fxa-settings/src/pages/mocks.tsx Adds PostVerifySetPasswordPage to shared MOCK_CMS_INFO.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 118 to 121
requiresKeys: () => true,
wantsKeys: () => true,
requiresPasswordForLogin: () => requiresPasswordForLogin,
getCmsInfo: () => cmsInfo,

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.

I had something like this originally, and then I opted to simplify it. I'd rather leave it as-is where you just pass in requiresPasswordForLogin depending on what you're testing.

Comment thread packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/index.test.tsx Outdated
…Sync isn't decoupled, service_welcome routing

Because:
- The CMS text for passwordless OTP users when Sync is not decoupled is not rendered on the set password page
- The repeat password field is not displayed when Sync is not decoupled
- Mobile users should not be navigated to the service welcome screen for VPN because the browser automatically closes it

This commit:
- Renders the CMS text and repeat password fields accordingly
- Returns a fixed requiresPasswordForLogin value from signup mock instead of rederiving it
- Adds a Mobile check and does not navigate those users to the service welcome screen

closes FXA-14047
closes FXA-14088
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.

2 participants