Skip to content

Commit 0309e3e

Browse files
committed
fix(splitLayout): Remove temp 'en-only' splitLayout guard
Because: * Split layout with English text in image didn't lead to a meaningful conversion, so we removed the images and want to display split layout to all users This commit: * Removes the "en-only" locale check that would conditionally show splitLayout if enabled or regular layout as a fallback closes FXA-13465
1 parent 27d7ad5 commit 0309e3e

2 files changed

Lines changed: 7 additions & 57 deletions

File tree

packages/fxa-settings/src/components/AppLayout/index.test.tsx

Lines changed: 2 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import {
1818
MOCK_CMS_INFO_SPLIT_LAYOUT_BG,
1919
} from './mocks';
2020
import { MOCK_CMS_INFO } from '../../pages/mocks';
21-
import { useDynamicLocalization } from '../../contexts/DynamicLocalizationContext';
2221

2322
// Mock the useConfig hook
2423
jest.mock('../../models/hooks', () => ({
@@ -32,17 +31,6 @@ jest.mock('../../models/hooks', () => ({
3231
}),
3332
}));
3433

35-
jest.mock('../../contexts/DynamicLocalizationContext', () => ({
36-
useDynamicLocalization: jest.fn(() => ({
37-
currentLocale: 'en',
38-
switchLanguage: jest.fn(),
39-
clearLanguagePreference: jest.fn(),
40-
isLoading: false,
41-
})),
42-
}));
43-
44-
const mockUseDynamicLocalization = useDynamicLocalization as jest.Mock;
45-
4634
describe('<AppLayout />', () => {
4735
it('renders as expected with children', async () => {
4836
renderWithLocalizationProvider(
@@ -393,15 +381,8 @@ describe('<AppLayout />', () => {
393381
});
394382
});
395383

396-
describe('splitLayout with locale restrictions (temp hack)', () => {
397-
it('renders split layout when splitLayout is true and locale is English', () => {
398-
mockUseDynamicLocalization.mockReturnValue({
399-
currentLocale: 'en-US',
400-
switchLanguage: jest.fn(),
401-
clearLanguagePreference: jest.fn(),
402-
isLoading: false,
403-
});
404-
384+
describe('splitLayout', () => {
385+
it('renders split layout when splitLayout is true', () => {
405386
renderWithLocalizationProvider(
406387
<AppLayout splitLayout={true} cmsInfo={MOCK_CMS_INFO_SPLIT_LAYOUT_BG}>
407388
<p>Split layout content</p>
@@ -417,31 +398,5 @@ describe('<AppLayout />', () => {
417398

418399
screen.getByText('Split layout content');
419400
});
420-
421-
it('renders default layout when splitLayout is true but locale is not English', () => {
422-
mockUseDynamicLocalization.mockReturnValue({
423-
currentLocale: 'fr',
424-
switchLanguage: jest.fn(),
425-
clearLanguagePreference: jest.fn(),
426-
isLoading: false,
427-
});
428-
429-
renderWithLocalizationProvider(
430-
<AppLayout splitLayout={true} cmsInfo={MOCK_CMS_INFO_HEADER_LOGO}>
431-
<p>Default layout content</p>
432-
</AppLayout>
433-
);
434-
435-
expect(
436-
screen.queryByRole('img', {
437-
name: MOCK_CMS_INFO_SPLIT_LAYOUT_BG.shared.backgrounds
438-
?.splitLayoutAltText as string,
439-
})
440-
).not.toBeInTheDocument();
441-
442-
screen.getByText('Default layout content');
443-
// CMS info is still applied even when split layout is disabled
444-
expect(screen.getByAltText('CMS Custom Logo')).toBeInTheDocument();
445-
});
446401
});
447402
});

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

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import { LocaleToggle } from '../LocaleToggle';
1313
import { DarkModeToggle } from '../DarkModeToggle';
1414
import { useConfig } from '../../models/hooks';
1515
import { CardLoadingSpinner } from '../CardLoadingSpinner';
16-
import { useDynamicLocalization } from '../../contexts/DynamicLocalizationContext';
1716

1817
type AppLayoutProps = {
1918
// TODO: FXA-6803 - the title prop should be made mandatory
@@ -55,20 +54,16 @@ export const AppLayout = ({
5554
}: AppLayoutProps) => {
5655
const { l10n } = useLocalization();
5756
const config = useConfig();
58-
const { currentLocale } = useDynamicLocalization();
59-
60-
// TEMP HACK (FXA-12988): Only show split layout for English locales
61-
const showSplitLayout = splitLayout && currentLocale.startsWith('en');
6257

6358
// Set the current page's layout preference in state so navigation
6459
// preserves the layout during Suspense fallback, preventing visual flash.
6560
// Only update if setCurrentSplitLayout is provided (it's omitted from Suspense fallback).
6661
// Uses useLayoutEffect instead of useEffect to prevent flicker of incorrect layout before paint
6762
useLayoutEffect(() => {
6863
if (setCurrentSplitLayout) {
69-
setCurrentSplitLayout(showSplitLayout);
64+
setCurrentSplitLayout(splitLayout);
7065
}
71-
}, [showSplitLayout, setCurrentSplitLayout]);
66+
}, [splitLayout, setCurrentSplitLayout]);
7267

7368
const cmsBackgrounds = cmsInfo?.shared?.backgrounds;
7469
const cmsPageTitle = cmsInfo?.shared?.pageTitle;
@@ -90,7 +85,7 @@ export const AppLayout = ({
9085
'flex min-h-screen flex-col items-center dark:bg-grey-900',
9186
cmsBackgrounds?.defaultLayout &&
9287
'mobileLandscape:[background:var(--cms-bg)]',
93-
showSplitLayout && 'mobileLandscape:relative'
88+
splitLayout && 'mobileLandscape:relative'
9489
)}
9590
style={
9691
cmsBackgrounds?.defaultLayout
@@ -108,7 +103,7 @@ export const AppLayout = ({
108103
cmsBackgrounds?.header &&
109104
'mobileLandscape:[background:var(--cms-header-bg)]',
110105
// Absolute position so the background-image can optionally show through.
111-
showSplitLayout && !cmsBackgrounds?.header && 'desktop:absolute'
106+
splitLayout && !cmsBackgrounds?.header && 'desktop:absolute'
112107
)}
113108
style={
114109
cmsBackgrounds?.header
@@ -143,7 +138,7 @@ export const AppLayout = ({
143138
</LinkExternal>
144139
</header>
145140

146-
{!showSplitLayout ? (
141+
{!splitLayout ? (
147142
<>
148143
<main className="flex mobileLandscape:items-center flex-1">
149144
<section>

0 commit comments

Comments
 (0)