Fix duplicate settings titles in page frame

Author: priscilawebdev

static/app/views/settings/components/settingsPageHeader.spec.tsx

@@ -0,0 +1,50 @@
+import {OrganizationFixture} from 'sentry-fixture/organization';
+
+import {render, screen, within} from 'sentry-test/reactTestingLibrary';
+
+import {useRoutes} from 'sentry/utils/useRoutes';
+import {TopBar} from 'sentry/views/navigation/topBar';
+import {SettingsBreadcrumb} from 'sentry/views/settings/components/settingsBreadcrumb';
+import {BreadcrumbProvider} from 'sentry/views/settings/components/settingsBreadcrumb/context';
+
+import {SettingsPageHeader} from './settingsPageHeader';
+
+jest.mock('sentry/utils/useRoutes');
+
+const mockUseRoutes = jest.mocked(useRoutes);
+
+describe('SettingsPageHeader', () => {
+  const routes = [
+    {name: 'One', path: '/one/'},
+    {name: 'Two', path: '/two/'},
+    {name: 'Three', path: '/three/'},
+  ];
+
+  beforeEach(() => {
+    mockUseRoutes.mockReturnValue(routes);
+  });
+
+  it('replaces the last breadcrumb in page-frame mode instead of rendering a duplicate title', () => {
+    render(
+      <TopBar.Slot.Provider>
+        <BreadcrumbProvider>
+          <TopBar.Slot name="title">
+            <SettingsBreadcrumb params={{}} routes={routes} />
+          </TopBar.Slot>
+          <TopBar.Slot.Outlet name="title">
+            {props => <div {...props} data-test-id="topbar-title-slot" />}
+          </TopBar.Slot.Outlet>
+          <SettingsPageHeader title="Projects" />
+        </BreadcrumbProvider>
+      </TopBar.Slot.Provider>,
+      {
+        organization: OrganizationFixture({features: ['page-frame']}),
+      }
+    );
+
+    const topbarTitleSlot = screen.getByTestId('topbar-title-slot');
+
+    expect(within(topbarTitleSlot).getAllByText('Projects')).toHaveLength(1);
+    expect(screen.queryByRole('heading', {name: 'Projects'})).not.toBeInTheDocument();
+  });
+});

static/app/views/settings/components/settingsPageHeader.tsx

@@ -1,8 +1,10 @@
 import styled from '@emotion/styled';
 
 import * as Layout from 'sentry/components/layouts/thirds';
+import {useRoutes} from 'sentry/utils/useRoutes';
 import {TopBar} from 'sentry/views/navigation/topBar';
 import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature';
+import {BreadcrumbTitle} from 'sentry/views/settings/components/settingsBreadcrumb/breadcrumbTitle';
 
 type Props = {
   /**
@@ -43,20 +45,34 @@ function UnstyledSettingsPageHeader({
   ...props
 }: Props) {
   const hasPageFrame = useHasPageFrameFeature();
+  const routes = useRoutes();
+  const hasTitle = title !== null && title !== undefined && title !== false;
+  const hasSubtitle = subtitle !== null && subtitle !== undefined && subtitle !== false;
+  const showVisibleTitle = hasTitle && !hasPageFrame;
+  const showHeaderText = showVisibleTitle || hasSubtitle;
   // If Header is narrow, use align-items to center <Action>.
   // Otherwise, use a fixed margin to prevent an odd alignment.
   // This is needed as Actions could be a button or a dropdown.
-  const isNarrow = !subtitle;
+  const isNarrow = !hasSubtitle;
+  const breadcrumbTitle =
+    hasPageFrame && typeof title === 'string' ? (
+      <BreadcrumbTitle routes={routes} title={title} />
+    ) : null;
 
   return (
     <div {...props}>
+      {breadcrumbTitle}
       <TitleAndActions isNarrow={isNarrow}>
         <TitleWrapper>
-          {icon && <Icon>{icon}</Icon>}
-          {title && (
+          {showHeaderText && icon && <Icon>{icon}</Icon>}
+          {showHeaderText && (
             <Title tabs={tabs} styled={noTitleStyles}>
-              <Layout.Title>{title}</Layout.Title>
-              {subtitle && <Subtitle colorSubtitle={colorSubtitle}>{subtitle}</Subtitle>}
+              {showVisibleTitle && <Layout.Title>{title}</Layout.Title>}
+              {hasSubtitle && (
+                <Subtitle colorSubtitle={colorSubtitle} hasTitle={showVisibleTitle}>
+                  {subtitle}
+                </Subtitle>
+              )}
             </Title>
           )}
         </TitleWrapper>
@@ -94,12 +110,12 @@ const Title = styled('div')<TitleProps>`
   margin: ${p => p.theme.space['3xl']} ${p => p.theme.space.xl}
     ${p => p.theme.space['2xl']} 0;
 `;
-const Subtitle = styled('div')<{colorSubtitle?: boolean}>`
+const Subtitle = styled('div')<{colorSubtitle?: boolean; hasTitle?: boolean}>`
   color: ${p =>
     p.colorSubtitle ? p.theme.tokens.content.accent : p.theme.colors.gray500};
   font-weight: ${p => p.theme.font.weight.sans.regular};
   font-size: ${p => p.theme.font.size.md};
-  padding: ${p => p.theme.space.lg} 0 0;
+  padding: ${p => (p.hasTitle ? `${p.theme.space.lg} 0 0` : 0)};
 `;
 
 const Icon = styled('div')`

static/app/views/settings/project/preprod/index.spec.tsx

@@ -0,0 +1,61 @@
+import {OrganizationFixture} from 'sentry-fixture/organization';
+
+import {render, screen} from 'sentry-test/reactTestingLibrary';
+
+import {TopBar} from 'sentry/views/navigation/topBar';
+
+import PreprodSettings from './index';
+
+jest.mock('sentry/views/preprod/components/preprodQuotaAlert', () => ({
+  PreprodQuotaAlert: () => <div data-test-id="preprod-quota-alert" />,
+}));
+
+jest.mock('./featureFilter', () => ({
+  FeatureFilter: () => <div data-test-id="feature-filter" />,
+}));
+
+jest.mock('./prCommentsToggle', () => ({
+  PrCommentsToggle: () => <div data-test-id="pr-comments-toggle" />,
+}));
+
+jest.mock('./snapshotPrCommentsToggle', () => ({
+  SnapshotPrCommentsToggle: () => <div data-test-id="snapshot-pr-comments-toggle" />,
+}));
+
+jest.mock('./snapshotStatusChecks', () => ({
+  SnapshotStatusChecks: () => <div data-test-id="snapshot-status-checks" />,
+}));
+
+jest.mock('./statusCheckRules', () => ({
+  StatusCheckRules: () => <div data-test-id="status-check-rules" />,
+}));
+
+describe('PreprodSettings', () => {
+  const pageDescription =
+    'Configure status checks and thresholds for your mobile build size analysis.';
+
+  it('renders the mobile builds title outside page-frame mode', () => {
+    render(<PreprodSettings />, {
+      organization: OrganizationFixture(),
+    });
+
+    expect(screen.getByRole('heading', {name: 'Mobile Builds'})).toBeInTheDocument();
+    expect(screen.getByText(pageDescription)).toBeInTheDocument();
+  });
+
+  it('hides the duplicate mobile builds title in page-frame mode', () => {
+    render(
+      <TopBar.Slot.Provider>
+        <PreprodSettings />
+      </TopBar.Slot.Provider>,
+      {
+        organization: OrganizationFixture({features: ['page-frame']}),
+      }
+    );
+
+    expect(
+      screen.queryByRole('heading', {name: 'Mobile Builds'})
+    ).not.toBeInTheDocument();
+    expect(screen.getByText(pageDescription)).toBeInTheDocument();
+  });
+});

static/app/views/settings/project/preprod/index.tsx

@@ -42,6 +42,9 @@ export default function PreprodSettings() {
   const navigate = useNavigate();
   const organization = useOrganization();
   const hasPageFrameFeature = useHasPageFrameFeature();
+  const pageDescription = t(
+    'Configure status checks and thresholds for your mobile build size analysis.'
+  );
 
   const hasSnapshots = organization.features.includes('preprod-snapshots');
 
@@ -62,9 +65,7 @@ export default function PreprodSettings() {
       <SentryDocumentTitle title={t('Mobile Builds')} />
       <SettingsPageHeader
         title={t('Mobile Builds')}
-        subtitle={t(
-          'Configure status checks and thresholds for your mobile build size analysis.'
-        )}
+        subtitle={pageDescription}
         action={
           <Grid flow="column" align="center" gap="lg">
             {hasPageFrameFeature ? (