fix(fxa-settings): Show "Sign in" heading on cached signin page instead of "Enter your password by vbudhram · Pull Request #20189 · mozilla/fxa

vbudhram · 2026-03-13T20:01:32Z

Because

The activePageCms selector used !isPasswordNeeded as a proxy for "is cached user," so cached users who need a password got SigninPage CMS content instead of SigninCachedPage
CardHeader renders empty <h1> and <p> when CMS page config is undefined but shared CMS props (logo, colors) are present

This pull request

Adds !hasCachedAccount guard to the heading condition in Signin/index.tsx so cached users always see "Sign in" heading (password input still renders when needed)
Changes activePageCms to select based on hasCachedAccount instead of !isPasswordNeeded
Updates CardHeader/index.tsx to fall back to headingText when cmsHeadline is undefined, and skip rendering <p> when cmsDescription is missing
Adds storybook story for cached user with CMS but no SigninCachedPage config

Issue

Closes: https://mozilla-hub.atlassian.net/browse/FXA-13274

Checklist

My commit is GPG signed
Tests pass locally (if applicable)
Documentation updated (if applicable)
RTL rendering verified (if UI changed)

Other Information

Looking at this a bit more, ISTM that we need a dedicated page for "cached" users. The current signin page handles both, but with cms overrides it is definately confusing.

packages/fxa-settings/src/pages/Signin/index.tsx

LZoog · 2026-03-13T21:21:07Z

packages/fxa-settings/src/pages/Signin/index.stories.tsx

+      cmsInfo: MOCK_CMS_INFO,
+    }),
+  },
+  'CMS > Regular layout > Cached > No SigninCachedPage config'


I think this story should probably explicitly set the page value to be undefined, otherwise if/when we add it to that mock object later this will no longer reflect the correct state.

packages/fxa-settings/src/pages/Signin/index.test.tsx

LZoog · 2026-03-17T20:16:12Z

packages/fxa-settings/src/pages/Signin/index.tsx

-  const title = activePageCms?.pageTitle;
-  const splitLayout = activePageCms?.splitLayout;
+  const title = (hasCachedAccount ? cachedPageCms : signinPageCms)?.pageTitle;
+  const splitLayout = (hasCachedAccount ? cachedPageCms : signinPageCms)


Okay I can see why you did the activePage thing now. I didn't see this before, that we need to pass in the button text and need to do a check regardless:

buttonText={ (hasCachedAccount ? cachedPageCms : signinPageCms) ?.primaryButtonText }

I reproduced the issue on main:

But in your branch currently, I see the headline issue is fixed, and there's now an issue with splitLayout (it's no showing even when it should) and the button text (showing the default instead of from the CMS). Page title probably has the same issue.

I think the check against hasCachedAccount is not the check that we want, it needs to match the check that we are doing for the headline... so Since the card header conditionally renders isPasswordNeeded && hasPassword, I think we need a new constant set like this:

const showCachedView = !(isPasswordNeeded && hasPassword);

And then you can update the CardHeader logic and base the conditional page CMS object on that.

Edit just for posterity it looks like isPasswordNeeded already checks hasPassword, so I think all we need is to check isPasswordNeeded instead of hasCachedAccount.

packages/fxa-settings/src/pages/Signin/index.test.tsx

LZoog · 2026-03-18T17:44:21Z

packages/fxa-settings/src/pages/Signin/index.tsx

-  // Use cached page config when available and user has cached account, else fall back to SigninPage
-  const activePageCms =
-    !isPasswordNeeded && cachedPageCms ? cachedPageCms : signinPageCms;
+  const activePageCms = isPasswordNeeded ? signinPageCms : cachedPageCms;


Can we go ahead and remove the hasPassword piece of isPasswordNeeded && hasPassword when rendering the card header, for consistency and since we don't need to check hasPassword any longer since it's already checked as part of isPasswordNeeded?

LZoog · 2026-03-18T17:57:51Z

packages/fxa-settings/src/pages/Signin/index.tsx

-    !isPasswordNeeded && cachedPageCms ? cachedPageCms : signinPageCms;
+  const activePageCms = isPasswordNeeded ? signinPageCms : cachedPageCms;
  const title = activePageCms?.pageTitle;
  const splitLayout = activePageCms?.splitLayout;


I mentioned this with the last review:

But in your branch currently, I see the headline issue is fixed, and there's now an issue with splitLayout (it's no showing even when it should) and the button text (showing the default instead of from the CMS). Page title probably has the same issue.

Unfortunately, this is still reproducing with latest. 😔

LZoog · 2026-03-18T17:59:05Z

packages/fxa-settings/src/components/CardHeader/index.tsx

+          {cmsHeadline || headingText}
        </h1>
-        <p className="card-subheader">{cmsDescription}</p>
+        {cmsDescription && (


I'm confused by this because the isCmsHeader check, already checks that this is not undefined, as well as cmsHeadline...

vbudhram · 2026-03-18T18:40:36Z

packages/fxa-settings/src/pages/Signin/index.tsx

  const title = activePageCms?.pageTitle;
-  const splitLayout = activePageCms?.splitLayout;
+  // If cachedPageCms is the active page but does not have a CMS entry,
+  // we reference the splitLayout property from the signinPageCms.


vbudhram · 2026-03-18T18:49:41Z

packages/fxa-settings/src/components/CardHeader/index.tsx

-    (props as CardHeaderCmsProps).cmsDescription !== undefined ||
-    (props as CardHeaderCmsProps).cmsHeadlineFontSize !== undefined ||
-    (props as CardHeaderCmsProps).cmsHeadlineTextColor !== undefined
+    (props as CardHeaderCmsProps).cmsHeadline !== undefined &&


I wanna say that product does not always define a description for cms pages in strapi, we might want to keep these all optional.

Copilot

Pull request overview

Adjusts FxA Settings sign-in CMS behavior so cached sign-in flows show the correct heading/content and avoids rendering empty header elements when CMS config is missing.

Changes:

Updates Signin page CMS selection logic and how CardHeader is populated for cached vs non-cached sign-in variants.
Makes RelierCmsInfo.SigninPage optional and updates related tests/stories/snapshots.
Adds a Storybook scenario for cached users when SigninCachedPage CMS config is missing.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
packages/fxa-settings/src/pages/Signin/index.tsx	Changes CMS page selection and which CMS props are passed into `CardHeader` for password-needed vs cached variants.
packages/fxa-settings/src/pages/Signin/index.test.tsx	Updates CMS-related assertions for cached/non-cached header behavior.
packages/fxa-settings/src/pages/Signin/index.stories.tsx	Adds Storybook story for cached flow with missing `SigninCachedPage` config.
packages/fxa-settings/src/pages/Signin/snapshots/index.test.tsx.snap	Removes/updates snapshot output impacted by header/logo rendering changes.
packages/fxa-settings/src/models/integrations/relier-interfaces.ts	Makes `SigninPage` optional on `RelierCmsInfo`.
packages/fxa-settings/src/components/CardHeader/index.tsx	Changes CMS-header detection logic (`isCmsHeader`).
packages/fxa-settings/src/components/CardHeader/index.test.tsx	Updates expectations for fallback behavior when only partial CMS props are provided.

Comments suppressed due to low confidence (1)

packages/fxa-settings/src/pages/Signin/index.tsx:438

The header selection is still driven only by isPasswordNeeded. Because isPasswordNeeded can be true even when hasCachedAccount is true (e.g. cached users when integration.wantsKeys() or wantsLogin()), cached users can still see the “Enter your password” header. If the goal is “cached users always see the Sign in heading while still rendering the password input when needed”, the conditional should incorporate hasCachedAccount (and keep password input rendering independent of the header choice).

      {isPasswordNeeded ? (
        <CardHeader
          headingText="Enter your password"
          headingAndSubheadingFtlId="signin-password-needed-header-2"
          {...{
            cmsLogoUrl: cmsInfo?.shared.logoUrl,
            cmsLogoAltText: cmsInfo?.shared.logoAltText,
            cmsHeadline: signinPageCms?.headline,
            cmsDescription: signinPageCms?.description,
            cmsHeadlineFontSize: cmsInfo?.shared.headlineFontSize,
            cmsHeadlineTextColor: cmsInfo?.shared.headlineTextColor,
          }}
        />
      ) : (
        <CardHeader
          headingText="Sign in"
          headingTextFtlId="signin-header"
          subheadingWithDefaultServiceFtlId="signin-subheader-without-logo-default"
          subheadingWithCustomServiceFtlId="signin-subheader-without-logo-with-servicename"
          {...{
            clientId,
            serviceName,
            cmsLogoUrl: cmsInfo?.shared.logoUrl,
            cmsLogoAltText: cmsInfo?.shared.logoAltText,
            cmsHeadline: cachedPageCms?.headline,
            cmsDescription: cachedPageCms?.description,
            cmsHeadlineFontSize: cmsInfo?.shared.headlineFontSize,
            cmsHeadlineTextColor: cmsInfo?.shared.headlineTextColor,
          }}
        />
      )}

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

LZoog · 2026-03-19T15:01:45Z

packages/fxa-settings/src/pages/Signin/index.tsx

  const cmsInfo = integration.getCmsInfo();
  const cachedPageCms = cmsInfo?.SigninCachedPage;
  const signinPageCms = cmsInfo?.SigninPage;
-  // Use cached page config when available and user has cached account, else fall back to SigninPage
-  const activePageCms =
-    !isPasswordNeeded && cachedPageCms ? cachedPageCms : signinPageCms;
+  const activePageCms = isPasswordNeeded ? signinPageCms : cachedPageCms;
  const title = activePageCms?.pageTitle;
-  const splitLayout = activePageCms?.splitLayout;
+  // If cachedPageCms is the active page but does not have a CMS entry,
+  // we reference the splitLayout property from the signinPageCms.
+  const splitLayout = activePageCms
+    ? activePageCms.splitLayout
+    : signinPageCms?.splitLayout;


“cached user” state is hasCachedAccount/sessionToken

This is not true when it comes to how we conditionally render the headline. We use isPasswordNeeded.

isPasswordNeeded can be false for non-cached passwordless accounts

This is wrong... hasPassword must be true in order for isPasswordNeeded to be true.

This is non-cached, in this PR.

packages/fxa-settings/src/pages/Signin/index.test.tsx

LZoog · 2026-03-19T14:56:52Z

packages/fxa-settings/src/components/CardHeader/index.tsx

-    (props as CardHeaderCmsProps).cmsHeadlineFontSize !== undefined ||
-    (props as CardHeaderCmsProps).cmsHeadlineTextColor !== undefined
-  );
+  return (props as CardHeaderCmsProps).cmsHeadline !== undefined;


If we don't want to require Product to enter cmsDescription / if that's already optional and they don't already enter it then yeah - let's not render the <p> tag for those cases.

LZoog · 2026-03-19T15:05:37Z

packages/fxa-settings/src/pages/Signin/index.tsx

+            cmsHeadline: signinPageCms?.headline,
+            cmsDescription: signinPageCms?.description,


Not sure why it's suggesting this change, that's definitely wrong. But if we don't have a test for wantsKeys that seems like we could add it.

…er description Simplify isCmsHeader predicate and fix CMS config selection for Signin pages. Conditionally render the CMS description <p> tag only when cmsDescription is defined, avoiding empty elements when Product omits the description field.

LZoog

👍

No cached signin set:

Then with it set:

vbudhram requested a review from a team as a code owner March 13, 2026 20:01

vbudhram force-pushed the fxa-13274 branch from 6a377f0 to 70838b0 Compare March 13, 2026 20:27

LZoog reviewed Mar 13, 2026

View reviewed changes

vbudhram force-pushed the fxa-13274 branch 3 times, most recently from 95954eb to 1beeb01 Compare March 14, 2026 04:55

vbudhram requested a review from LZoog March 16, 2026 15:39

vbudhram force-pushed the fxa-13274 branch 3 times, most recently from 484abbe to 24a1c99 Compare March 16, 2026 17:01

LZoog reviewed Mar 17, 2026

View reviewed changes

vbudhram force-pushed the fxa-13274 branch 2 times, most recently from bdd38c2 to a20a4d7 Compare March 18, 2026 15:52

LZoog reviewed Mar 18, 2026

View reviewed changes

vbudhram commented Mar 18, 2026

View reviewed changes

vbudhram force-pushed the fxa-13274 branch from 06115be to 044ceaa Compare March 18, 2026 18:41

vbudhram commented Mar 18, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings March 19, 2026 01:40

vbudhram force-pushed the fxa-13274 branch from 044ceaa to f18c217 Compare March 19, 2026 01:40

Copilot AI reviewed Mar 19, 2026

View reviewed changes

vbudhram force-pushed the fxa-13274 branch from dd1b353 to 2b10a96 Compare March 19, 2026 16:20

LZoog approved these changes Mar 19, 2026

View reviewed changes

vbudhram merged commit eb76533 into main Mar 19, 2026
22 checks passed

vbudhram deleted the fxa-13274 branch March 19, 2026 19:11

		cmsHeadline: signinPageCms?.headline,
		cmsDescription: signinPageCms?.description,

Conversation

vbudhram commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Because

This pull request

Issue

Checklist

Other Information

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LZoog Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbudhram Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LZoog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vbudhram commented Mar 13, 2026 •

edited

Loading

LZoog Mar 17, 2026 •

edited

Loading

vbudhram Mar 18, 2026 •

edited

Loading