diff --git a/static/app/router/routes.spec.tsx b/static/app/router/routes.spec.tsx index 55cb251fdc14..12358079dc76 100644 --- a/static/app/router/routes.spec.tsx +++ b/static/app/router/routes.spec.tsx @@ -116,7 +116,6 @@ describe('buildRoutes()', () => { { type: 'project', to: '/settings/:orgId/projects/:projectId/', - switchTo: '/settings/:orgId/projects/:projectId/keys/', }, { type: 'link', diff --git a/static/app/router/routes.tsx b/static/app/router/routes.tsx index 5bf37e40edee..67968c69189d 100644 --- a/static/app/router/routes.tsx +++ b/static/app/router/routes.tsx @@ -533,8 +533,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', - switchTo: '/settings/:orgId/projects/:projectId/', }, + settingsProjectSwitch: {switchTo: '/settings/:orgId/projects/:projectId/'}, }, index: true, name: t('General'), @@ -549,8 +549,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', - switchTo: '/settings/:orgId/projects/:projectId/teams/', }, + settingsProjectSwitch: {switchTo: '/settings/:orgId/projects/:projectId/teams/'}, }, path: 'teams/', name: t('Teams'), @@ -561,8 +561,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', - switchTo: '/settings/:orgId/projects/:projectId/alerts/', }, + settingsProjectSwitch: {switchTo: '/settings/:orgId/projects/:projectId/alerts/'}, }, path: 'alerts/', name: t('Alerts'), @@ -603,6 +603,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/attributes/', }, }, @@ -615,8 +617,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', - switchTo: '/settings/:orgId/projects/:projectId/tags/', }, + settingsProjectSwitch: {switchTo: '/settings/:orgId/projects/:projectId/tags/'}, }, path: 'tags/', name: t('Tags & Context'), @@ -627,6 +629,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/environments/', }, }, @@ -647,6 +651,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/ownership/', }, }, @@ -663,6 +669,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/user-feedback/', }, }, @@ -675,6 +683,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/toolbar/', }, }, @@ -687,6 +697,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/filters/', }, }, @@ -708,6 +720,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/security-and-privacy/', }, }, @@ -733,6 +747,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/issue-grouping/', }, }, @@ -745,6 +761,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/debug-symbols/', }, }, @@ -757,6 +775,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/proguard/', }, }, @@ -769,6 +789,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/source-maps/', }, }, @@ -806,6 +828,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/performance/', }, }, @@ -822,6 +846,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/replays/', }, }, @@ -834,8 +860,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', - switchTo: '/settings/:orgId/projects/:projectId/logs/', }, + settingsProjectSwitch: {switchTo: '/settings/:orgId/projects/:projectId/logs/'}, }, path: 'logs/', name: t('Logs'), @@ -846,6 +872,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/playstation/', }, }, @@ -858,6 +886,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/mobile-builds/', }, }, @@ -870,6 +900,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/snapshots/', }, }, @@ -882,8 +914,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', - switchTo: '/settings/:orgId/projects/:projectId/keys/', }, + settingsProjectSwitch: {switchTo: '/settings/:orgId/projects/:projectId/keys/'}, }, path: 'keys/', name: t('Client Keys'), @@ -913,6 +945,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/loader-script/', }, }, @@ -925,6 +959,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/release-tracking/', }, }, @@ -939,6 +975,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/security-headers/', }, }, @@ -974,6 +1012,8 @@ function buildRoutes(): RouteObject[] { settingsBreadcrumb: { type: 'project', to: '/settings/:orgId/projects/:projectId/', + }, + settingsProjectSwitch: { switchTo: '/settings/:orgId/projects/:projectId/legacy-webhooks/', }, }, diff --git a/static/app/router/types.tsx b/static/app/router/types.tsx index e7506754e034..8d61818f6b7b 100644 --- a/static/app/router/types.tsx +++ b/static/app/router/types.tsx @@ -1,4 +1,5 @@ import type {SettingsBreadcrumb} from 'sentry/views/settings/components/settingsBreadcrumb/types'; +import type {SettingsRouteHandle} from 'sentry/views/settings/types'; /** * This is our "custom" route object. It varies a bit from React Router's @@ -15,7 +16,8 @@ interface BaseRouteObject { */ customerDomainOnlyRoute?: true; - handle?: Record & {settingsBreadcrumb?: SettingsBreadcrumb}; + handle?: Record & + SettingsRouteHandle & {settingsBreadcrumb?: SettingsBreadcrumb}; /** * Is a index route */ diff --git a/static/app/utils/useReturnPath.spec.tsx b/static/app/utils/useReturnPath.spec.tsx new file mode 100644 index 000000000000..05862b5702f0 --- /dev/null +++ b/static/app/utils/useReturnPath.spec.tsx @@ -0,0 +1,48 @@ +import {renderHookWithProviders} from 'sentry-test/reactTestingLibrary'; + +import {getReturnPath, useCaptureReturnPath} from 'sentry/utils/useReturnPath'; + +const KEY = 'test-return-path'; +const options = {key: KEY, excludePrefix: '/settings'}; + +describe('useReturnPath', () => { + beforeEach(() => { + sessionStorage.clear(); + }); + + it('captures locations outside the excluded prefix', () => { + renderHookWithProviders(() => useCaptureReturnPath(options), { + initialRouterConfig: { + location: {pathname: '/issues/', query: {query: 'is:unresolved'}}, + }, + }); + + expect(getReturnPath(KEY, '/fallback/')).toBe('/issues/?query=is%3Aunresolved'); + }); + + it('does not capture without a key', () => { + renderHookWithProviders( + () => useCaptureReturnPath({key: null, excludePrefix: '/settings'}), + {initialRouterConfig: {location: {pathname: '/issues/'}}} + ); + + expect(getReturnPath(KEY, '/fallback/')).toBe('/fallback/'); + }); + + it('does not capture locations under the excluded prefix', () => { + renderHookWithProviders(() => useCaptureReturnPath(options), { + initialRouterConfig: {location: {pathname: '/settings/org-slug/members/'}}, + }); + + expect(getReturnPath(KEY, '/fallback/')).toBe('/fallback/'); + }); + + it.each(['//attacker.example/', '/\\attacker.example/', 'https://attacker.example/'])( + 'falls back instead of returning to another site via %s', + path => { + sessionStorage.setItem(KEY, path); + + expect(getReturnPath(KEY, '/fallback/')).toBe('/fallback/'); + } + ); +}); diff --git a/static/app/utils/useReturnPath.ts b/static/app/utils/useReturnPath.ts new file mode 100644 index 000000000000..d1dc0eaa6fe9 --- /dev/null +++ b/static/app/utils/useReturnPath.ts @@ -0,0 +1,50 @@ +import {useEffect} from 'react'; +import {matchPath} from 'react-router'; + +import {sessionStorageWrapper} from 'sentry/utils/sessionStorage'; +import {useLocation} from 'sentry/utils/useLocation'; + +interface CaptureReturnPathOptions { + /** + * Locations under this path prefix (e.g. `/settings`) are not captured. + */ + excludePrefix: string; + /** + * The session storage key the return path is stored under. Nothing is + * captured while this is null, e.g. when the key depends on something that + * is not known yet. + */ + key: string | null; +} + +/** + * Remembers the last page the user visited outside of a section of the app, so + * a "back" control inside that section can return there. Browser history is + * not reliable for this because users navigate between several pages of the + * section before leaving. + * + * Must be mounted outside of the section it excludes. + */ +export function useCaptureReturnPath({key, excludePrefix}: CaptureReturnPathOptions) { + const {pathname, search, hash} = useLocation(); + + useEffect(() => { + if (key === null || matchPath({path: excludePrefix, end: false}, pathname)) { + return; + } + sessionStorageWrapper.setItem(key, `${pathname}${search}${hash}`); + }, [key, excludePrefix, pathname, search, hash]); +} + +// A path on this site. Links treat `//host` and `/\host` as another site, and +// unmatched routes can capture such a pathname. +const SAME_ORIGIN_PATH = /^\/(?![/\\])/; + +/** + * The page captured by `useCaptureReturnPath`, or `fallback` when none was + * captured, e.g. when the section was entered directly. + */ +export function getReturnPath(key: string, fallback: string): string { + const path = sessionStorageWrapper.getItem(key); + return path && SAME_ORIGIN_PATH.test(path) ? path : fallback; +} diff --git a/static/app/views/navigation/constants.tsx b/static/app/views/navigation/constants.tsx index 1863652cbea0..61bd1b81c52a 100644 --- a/static/app/views/navigation/constants.tsx +++ b/static/app/views/navigation/constants.tsx @@ -26,3 +26,11 @@ export const TOP_BAR_HEIGHT_CSS_VAR = '--top-bar-height'; // To be passed as the `source` parameter in router navigation state // e.g. {pathname: '/issues/', state: {source: `sidebar`}} export const SIDEBAR_NAVIGATION_SOURCE = 'sidebar'; + +// Settings has its own navigation, so "Back to app" returns to the last page +// visited outside of this path prefix. +export const SETTINGS_PATH_PREFIX = '/settings'; + +// Scoped by organization so "Back to app" never returns to another organization. +export const getSettingsReturnPathKey = (organizationSlug: string) => + `settings-return-path:${organizationSlug}`; diff --git a/static/app/views/navigation/index.desktop.spec.tsx b/static/app/views/navigation/index.desktop.spec.tsx index 4aed0e7c81a1..d8c12e16b841 100644 --- a/static/app/views/navigation/index.desktop.spec.tsx +++ b/static/app/views/navigation/index.desktop.spec.tsx @@ -15,6 +15,7 @@ import { } from 'sentry-test/reactTestingLibrary'; import {ConfigStore} from 'sentry/stores/configStore'; +import {ProjectsStore} from 'sentry/stores/projectsStore'; import {Navigation} from 'sentry/views/navigation'; import {NAVIGATION_SIDEBAR_COLLAPSED_LOCAL_STORAGE_KEY} from 'sentry/views/navigation/constants'; import {PrimaryNavigationContextProvider} from 'sentry/views/navigation/primaryNavigationContext'; @@ -98,7 +99,9 @@ function assertValidListHTML(list: HTMLElement) { } function setupMocks() { + ProjectsStore.reset(); localStorage.clear(); + sessionStorage.clear(); MockApiClient.clearMockResponses(); ConfigStore.set('user', UserFixture()); @@ -222,6 +225,225 @@ describe('desktop navigation', () => { ); }); + describe('settings v3', () => { + const settingsV3 = {features: [...ALL_AVAILABLE_FEATURES, 'settings-v3']}; + + beforeEach(() => { + ProjectsStore.loadInitialData([ + ProjectFixture(), + ProjectFixture({id: '999', slug: 'second-project'}), + ]); + MockApiClient.addMockResponse({ + url: '/projects/org-slug/second-project/', + body: ProjectFixture({id: '999', slug: 'second-project'}), + }); + }); + + it('shows the settings scope rail', async () => { + render( + + + , + navigationContext({ + organization: settingsV3, + initialRouterConfig: {location: {pathname: '/settings/org-slug/'}}, + }) + ); + + assertActivePrimaryNavLink(screen.getByRole('link', {name: 'Org'})); + await waitFor(() => + expect(screen.getByRole('link', {name: 'Project'})).toHaveAttribute( + 'href', + '/settings/org-slug/projects/project-slug/' + ) + ); + expect(screen.getByRole('link', {name: 'Account'})).toHaveAttribute( + 'href', + '/settings/account/' + ); + expect(screen.getByRole('button', {name: 'Back to app'})).toBeInTheDocument(); + expect(screen.queryByRole('link', {name: 'Issues'})).not.toBeInTheDocument(); + }); + + it('shows organization settings without account pages', () => { + render( + + + , + navigationContext({ + organization: settingsV3, + initialRouterConfig: {location: {pathname: '/settings/org-slug/'}}, + }) + ); + + expect(screen.getByText('Org Settings')).toBeInTheDocument(); + expect(screen.getByRole('link', {name: 'Organization Tokens'})).toBeInTheDocument(); + expect( + screen.queryByRole('link', {name: 'Personal Tokens'}) + ).not.toBeInTheDocument(); + }); + + it('shows account settings under the account scope', async () => { + const {router} = render( + + + , + navigationContext({ + organization: settingsV3, + initialRouterConfig: {location: {pathname: '/settings/org-slug/'}}, + }) + ); + + await userEvent.click(screen.getByRole('link', {name: 'Account'})); + + expect(router.location.pathname).toBe('/settings/account/'); + assertActivePrimaryNavLink(screen.getByRole('link', {name: 'Account'})); + expect(screen.getByText('Account Settings')).toBeInTheDocument(); + expect(screen.getByRole('link', {name: 'General'})).toBeInTheDocument(); + expect(screen.getByRole('link', {name: 'Personal Tokens'})).toBeInTheDocument(); + }); + + it.each([ + {customerDomain: false, pathname: '/settings/org-slug/projects/'}, + {customerDomain: true, pathname: '/settings/projects/'}, + ])( + 'opens and switches project settings at $pathname', + async ({customerDomain, pathname}) => { + mockUsingCustomerDomain.mockReturnValue(customerDomain); + if (customerDomain) { + ConfigStore.set('customerDomain', { + subdomain: 'org-slug', + organizationUrl: 'https://org-slug.sentry.io', + sentryUrl: 'https://sentry.io', + }); + } + const {router} = render( + + + , + navigationContext({ + organization: settingsV3, + initialRouterConfig: { + location: {pathname}, + route: `${pathname}:projectId?/*`, + }, + }) + ); + + await userEvent.click(screen.getByRole('link', {name: 'Project'})); + await waitFor(() => + expect(router.location.pathname).toBe(`${pathname}project-slug/`) + ); + assertActivePrimaryNavLink(screen.getByRole('link', {name: 'Project'})); + assertActiveSecondaryNavLink(screen.getByRole('link', {name: 'General'})); + + // Switching projects keeps the current settings page + await userEvent.click(screen.getByRole('link', {name: 'Project Teams'})); + await userEvent.click(screen.getByRole('button', {name: 'Select a project'})); + await userEvent.click( + await screen.findByRole('option', {name: 'second-project'}) + ); + expect(router.location.pathname).toBe(`${pathname}second-project/teams/`); + expect(screen.getByRole('link', {name: 'Project'})).toHaveAttribute( + 'href', + `${pathname}second-project/` + ); + } + ); + + it('switches from project key details to the new project key list', async () => { + const {router} = render( + + + , + navigationContext({ + organization: settingsV3, + initialRouterConfig: { + location: {pathname: '/settings/org-slug/projects/project-slug/keys/123/'}, + route: '/settings/:orgId/projects/:projectId/', + children: [ + { + path: 'keys/', + handle: { + settingsProjectSwitch: { + switchTo: '/settings/:orgId/projects/:projectId/keys/', + }, + }, + children: [ + {path: ':keyId/', element:
}, + {index: true, element:
}, + ], + }, + ], + }, + }) + ); + + await userEvent.click(screen.getByRole('button', {name: 'Select a project'})); + await userEvent.click(await screen.findByRole('option', {name: 'second-project'})); + expect(router.location.pathname).toBe( + '/settings/org-slug/projects/second-project/keys/' + ); + }); + + it('returns to the page settings was entered from', async () => { + const {router} = render( + + + , + navigationContext({organization: settingsV3}) + ); + + await userEvent.click(screen.getByRole('link', {name: 'Explore'})); + const exploreLocation = router.location.pathname; + await userEvent.click( + screen.getByRole('button', {name: 'Toggle organization menu'}) + ); + await userEvent.click(await screen.findByRole('menuitemradio', {name: 'Settings'})); + // Visit another settings page, so browser history alone would not return + await userEvent.click(screen.getByRole('link', {name: 'Account'})); + + await userEvent.click(screen.getByRole('button', {name: 'Back to app'})); + expect(router.location.pathname).toBe(exploreLocation); + }); + + it('does not return to a page from another organization', async () => { + sessionStorage.setItem( + 'settings-return-path:other-org', + '/organizations/other-org/explore/traces/' + ); + const {router} = render( + + + , + navigationContext({ + organization: settingsV3, + initialRouterConfig: {location: {pathname: '/settings/org-slug/'}}, + }) + ); + + await userEvent.click(screen.getByRole('button', {name: 'Back to app'})); + expect(router.location.pathname).toBe('/organizations/org-slug/issues/'); + }); + + it('does not title the admin navigation as org settings', async () => { + render( + + + , + navigationContext({ + organization: settingsV3, + initialRouterConfig: {location: {pathname: '/manage/'}}, + }) + ); + + expect( + await screen.findByRole('link', {name: 'Organizations'}) + ).toBeInTheDocument(); + expect(screen.queryByText('Org Settings')).not.toBeInTheDocument(); + }); + }); + it('reopens collapsed account navigation without an organization', async () => { localStorage.setItem(NAVIGATION_SIDEBAR_COLLAPSED_LOCAL_STORAGE_KEY, 'true'); diff --git a/static/app/views/navigation/index.mobile.spec.tsx b/static/app/views/navigation/index.mobile.spec.tsx index 927e27af9748..330666d44186 100644 --- a/static/app/views/navigation/index.mobile.spec.tsx +++ b/static/app/views/navigation/index.mobile.spec.tsx @@ -52,6 +52,7 @@ function navigationContext({ function setupMocks() { localStorage.clear(); + sessionStorage.clear(); MockApiClient.clearMockResponses(); // MobileNavigation requires a #main element to manage inert/overflow attributes @@ -98,6 +99,64 @@ describe('mobile navigation', () => { document.getElementById('main')?.remove(); }); + describe('settings v3', () => { + const settingsV3 = {features: [...ALL_AVAILABLE_FEATURES, 'settings-v3']}; + + it('uses the settings scope rail', async () => { + const {router} = render( + + + , + navigationContext({ + organization: settingsV3, + initialRouterConfig: {location: {pathname: '/settings/org-slug/'}}, + }) + ); + + await userEvent.click(screen.getByRole('button', {name: 'Open main menu'})); + expect(screen.getByRole('link', {name: 'Org'})).toHaveAttribute( + 'aria-current', + 'location' + ); + expect(screen.queryByRole('link', {name: 'Issues'})).not.toBeInTheDocument(); + + await userEvent.click(screen.getByRole('link', {name: 'Account'})); + expect(router.location.pathname).toBe('/settings/account/'); + expect(screen.getByRole('link', {name: 'Account'})).toHaveAttribute( + 'aria-current', + 'location' + ); + expect(screen.getByRole('link', {name: 'General'})).toBeInTheDocument(); + + await userEvent.click(screen.getByRole('button', {name: 'Collapse'})); + expect( + screen.queryByRole('navigation', {name: 'Primary Navigation'}) + ).not.toBeInTheDocument(); + }); + + it('returns to the page settings was entered from through the menu', async () => { + const {router} = render( + + + , + navigationContext({organization: settingsV3}) + ); + + await userEvent.click(screen.getByRole('button', {name: 'Open main menu'})); + await userEvent.click(screen.getByRole('link', {name: 'Explore'})); + await userEvent.click( + screen.getByRole('button', {name: 'Toggle organization menu'}) + ); + await userEvent.click(await screen.findByRole('menuitemradio', {name: 'Settings'})); + // The Explore tap above must not leave its secondary navigation behind + expect(screen.getByText('Org Settings')).toBeInTheDocument(); + + await userEvent.click(screen.getByRole('button', {name: 'Back to app'})); + expect(router.location.pathname).toMatch(/^\/organizations\/org-slug\/explore\//); + expect(screen.queryByText('Org Settings')).not.toBeInTheDocument(); + }); + }); + it('navigates account settings without an organization', async () => { document.getElementById('main')?.remove(); diff --git a/static/app/views/navigation/index.tsx b/static/app/views/navigation/index.tsx index db31fce8617f..3a27b6b47162 100644 --- a/static/app/views/navigation/index.tsx +++ b/static/app/views/navigation/index.tsx @@ -12,6 +12,11 @@ import {CommandPaletteSlot} from 'sentry/components/commandPalette/ui/commandPal import {CommandPaletteHotkeys} from 'sentry/components/commandPalette/ui/commandPaletteStateContext'; import {t} from 'sentry/locale'; import {useOrganization} from 'sentry/utils/useOrganization'; +import {useCaptureReturnPath} from 'sentry/utils/useReturnPath'; +import { + SETTINGS_PATH_PREFIX, + getSettingsReturnPathKey, +} from 'sentry/views/navigation/constants'; import {MobileNavigation} from 'sentry/views/navigation/mobileNavigation'; import {Navigation as DesktopNavigation} from 'sentry/views/navigation/navigation'; import { @@ -55,6 +60,11 @@ function UserAndOrganizationNavigation({pageBannerHeight}: NavigationProps) { const {visible} = useModal(); const {view, setView} = useSecondaryNavigation(); + useCaptureReturnPath({ + key: organization ? getSettingsReturnPathKey(organization.slug) : null, + excludePrefix: SETTINGS_PATH_PREFIX, + }); + useHotkeys( visible ? [] diff --git a/static/app/views/navigation/mobileNavigation.tsx b/static/app/views/navigation/mobileNavigation.tsx index 7fd372d203af..999fbe0c9795 100644 --- a/static/app/views/navigation/mobileNavigation.tsx +++ b/static/app/views/navigation/mobileNavigation.tsx @@ -20,6 +20,8 @@ import { PrimaryNavigationFooterItems, PrimaryNavigationFooterItemsUserDropdown, PrimaryNavigationItems, + SettingsNavigationBackButton, + SettingsPrimaryNavigationItems, } from 'sentry/views/navigation/navigation'; import {PrimaryNavigation} from 'sentry/views/navigation/primary/components'; import { @@ -31,6 +33,7 @@ import {SearchButton} from 'sentry/views/navigation/searchButton'; import {SecondaryNavigation} from 'sentry/views/navigation/secondary/components'; import {SecondaryNavigationContent} from 'sentry/views/navigation/secondary/content'; import {useSecondaryNavigation} from 'sentry/views/navigation/secondaryNavigationContext'; +import {useSettingsNavigation} from 'sentry/views/navigation/useSettingsNavigation'; function MobileNavigationHeader(props: FlexProps<'header'>) { const theme = useTheme(); @@ -66,16 +69,26 @@ function MobileHelpMenuFallback() { function MobilePrimaryNavigation() { const organization = useOrganization({allowNull: true}); + const isSettingsNavigation = useSettingsNavigation(); const {view} = useSecondaryNavigation(); return ( - {organization && } + {organization && + (isSettingsNavigation ? ( + + ) : ( + + ))} - + {isSettingsNavigation && organization ? ( + + ) : ( + + )} {view === 'expanded' && ( diff --git a/static/app/views/navigation/navigation.tsx b/static/app/views/navigation/navigation.tsx index 9ab67284b836..394475575a49 100644 --- a/static/app/views/navigation/navigation.tsx +++ b/static/app/views/navigation/navigation.tsx @@ -1,14 +1,19 @@ import {Fragment, type PropsWithChildren, type RefObject, useMemo, useRef} from 'react'; import {mergeProps} from '@react-aria/utils'; +import {IconArrow} from '@sentry/icons/arrow'; +import {IconBuilding} from '@sentry/icons/building'; import {IconCompass} from '@sentry/icons/compass'; import {IconDashboard} from '@sentry/icons/dashboard'; import {IconGraph} from '@sentry/icons/graph'; import {IconIssues} from '@sentry/icons/issues'; +import {IconProject} from '@sentry/icons/project'; import {IconSeer} from '@sentry/icons/seer'; import {IconSettings} from '@sentry/icons/settings'; import {IconSiren} from '@sentry/icons/siren'; +import {IconUser} from '@sentry/icons/user'; import {motion, type MotionProps} from 'framer-motion'; +import {LinkButton} from '@sentry/scraps/button'; import {Stack, Flex} from '@sentry/scraps/layout'; import {SizeProvider} from '@sentry/scraps/sizeContext'; @@ -17,6 +22,8 @@ import {ErrorBoundary} from 'sentry/components/errorBoundary'; import {Override} from 'sentry/components/override'; import {t} from 'sentry/locale'; import {useOrganization} from 'sentry/utils/useOrganization'; +import {useProjects} from 'sentry/utils/useProjects'; +import {getReturnPath} from 'sentry/utils/useReturnPath'; import {useSyncedLocalStorageState} from 'sentry/utils/useSyncedLocalStorageState'; import {getDefaultExploreRoute} from 'sentry/views/explore/utils'; import { @@ -27,6 +34,7 @@ import { NAVIGATION_SIDEBAR_SECONDARY_WIDTH_LOCAL_STORAGE_KEY, PRIMARY_SIDEBAR_WIDTH, SECONDARY_SIDEBAR_WIDTH, + getSettingsReturnPathKey, } from 'sentry/views/navigation/constants'; import { NavigationTour, @@ -47,16 +55,19 @@ import {SecondaryNavigation} from 'sentry/views/navigation/secondary/components' import {SecondaryNavigationContent} from 'sentry/views/navigation/secondary/content'; import {useSecondaryNavigation} from 'sentry/views/navigation/secondaryNavigationContext'; import {useCollapsedNavigation} from 'sentry/views/navigation/useCollapsedNavigation'; +import {useSettingsNavigation} from 'sentry/views/navigation/useSettingsNavigation'; import {useLLMContext} from 'sentry/views/seerExplorer/contexts/llmContext'; import {registerLLMContext} from 'sentry/views/seerExplorer/contexts/registerLLMContext'; import { getAutomationsPath, useAutomationsEnabled, } from 'sentry/views/seerWorkflows/utils'; +import {useSettingsScope} from 'sentry/views/settings/useSettingsScope'; import {useSettingsV3} from 'sentry/views/settings/useSettingsV3'; function NavigationImpl() { const organization = useOrganization({allowNull: true}); + const isSettingsNavigation = useSettingsNavigation(); const collapsedNavigation = useCollapsedNavigation(); const {view} = useSecondaryNavigation(); @@ -100,10 +111,20 @@ function NavigationImpl() { - {organization ? : } + {isSettingsNavigation && organization ? ( + + ) : organization ? ( + + ) : ( + + )} - + {isSettingsNavigation && organization ? ( + + ) : ( + + )} {organization && ( @@ -155,6 +176,66 @@ function NavigationImpl() { export const Navigation = registerLLMContext('navigation', NavigationImpl); +export function SettingsNavigationBackButton({ + organizationSlug, +}: { + organizationSlug: string; +}) { + const {setActiveGroup} = usePrimaryNavigation(); + return ( + } + aria-label={t('Back to app')} + tooltipProps={{title: t('Back to app')}} + to={getReturnPath( + getSettingsReturnPathKey(organizationSlug), + `/organizations/${organizationSlug}/issues/` + )} + onClick={() => setActiveGroup(null)} + /> + ); +} + +export function SettingsPrimaryNavigationItems({ + organizationSlug, +}: { + organizationSlug: string; +}) { + const {projects} = useProjects(); + const {scope: activeScope, projectSlug: activeProjectSlug} = useSettingsScope(); + const projectSlug = activeProjectSlug ?? projects[0]?.slug; + const projectSettingsPath = `/settings/${organizationSlug}/projects/`; + const items = [ + { + key: 'org', + label: t('Org'), + to: `/settings/${organizationSlug}/`, + icon: , + }, + { + key: 'project', + label: t('Project'), + to: projectSlug ? `${projectSettingsPath}${projectSlug}/` : projectSettingsPath, + icon: , + }, + {key: 'account', label: t('Account'), to: '/settings/account/', icon: }, + ]; + + return items.map(({key, label, to, icon}) => ( + + + {icon} + + + )); +} + interface PrimaryNavigationItemsProps { listRef?: RefObject; } diff --git a/static/app/views/navigation/primary/components.tsx b/static/app/views/navigation/primary/components.tsx index 9e448177f0d7..54e236ebb229 100644 --- a/static/app/views/navigation/primary/components.tsx +++ b/static/app/views/navigation/primary/components.tsx @@ -132,8 +132,6 @@ interface PrimaryNavigationLinkProps function PrimaryNavigationLink(props: PrimaryNavigationLinkProps) { const organization = useOrganization({allowNull: true}); - const {layout, features} = usePrimaryNavigation(); - const isMobile = layout === 'mobile'; // Reload the page when the frontend is stale to ensure users get the latest version const {state: appState} = useFrontendVersion(); const theme = useTheme(); @@ -147,10 +145,6 @@ function PrimaryNavigationLink(props: PrimaryNavigationLinkProps) { onMouseEnter: props.onMouseEnter, onMouseLeave: props.onMouseLeave, onClick: (e: React.MouseEvent) => { - // On touch mobile devices, prevent navigation and let setActiveGroup handle the active state - if (isMobile && !features.hover) { - e.preventDefault(); - } trackAnalytics('navigation.primary_item_clicked', { item: props.analyticsKey, organization, diff --git a/static/app/views/navigation/primary/useActivateNavigationGroupOnHover.tsx b/static/app/views/navigation/primary/useActivateNavigationGroupOnHover.tsx index f6a157a69347..3eb609a43991 100644 --- a/static/app/views/navigation/primary/useActivateNavigationGroupOnHover.tsx +++ b/static/app/views/navigation/primary/useActivateNavigationGroupOnHover.tsx @@ -31,7 +31,7 @@ interface UseActivateNavigationGroupOnHoverProps { export function useActivateNavigationGroupOnHover({ ref, }: UseActivateNavigationGroupOnHoverProps) { - const {layout, setActiveGroup, activeGroup} = usePrimaryNavigation(); + const {layout, features, setActiveGroup, activeGroup} = usePrimaryNavigation(); const {view} = useSecondaryNavigation(); const location = useLocation(); @@ -117,7 +117,11 @@ export function useActivateNavigationGroupOnHover({ } }; - const onClick = () => { + const onClick = (e: React.MouseEvent) => { + // On touch mobile devices, activate the group without navigating. + if (layout === 'mobile' && !features.hover) { + e.preventDefault(); + } setActiveGroup(group); }; diff --git a/static/app/views/navigation/primaryNavigationContext.spec.tsx b/static/app/views/navigation/primaryNavigationContext.spec.tsx new file mode 100644 index 000000000000..5e85f699b349 --- /dev/null +++ b/static/app/views/navigation/primaryNavigationContext.spec.tsx @@ -0,0 +1,44 @@ +import {act, renderHookWithProviders} from 'sentry-test/reactTestingLibrary'; + +import { + PrimaryNavigationContextProvider, + usePrimaryNavigation, +} from 'sentry/views/navigation/primaryNavigationContext'; + +describe('PrimaryNavigationContextProvider', () => { + it('uses the group of the current route by default', () => { + const {result} = renderHookWithProviders(usePrimaryNavigation, { + additionalWrapper: PrimaryNavigationContextProvider, + initialRouterConfig: {location: {pathname: '/organizations/org-slug/issues/'}}, + }); + + expect(result.current.activeGroup).toBe('issues'); + }); + + it('lets the active group be overridden within the current route group', () => { + const {result} = renderHookWithProviders(usePrimaryNavigation, { + additionalWrapper: PrimaryNavigationContextProvider, + initialRouterConfig: {location: {pathname: '/organizations/org-slug/issues/'}}, + }); + + act(() => result.current.setActiveGroup('explore')); + + expect(result.current.activeGroup).toBe('explore'); + }); + + it('drops the override once the route group changes, even when returning', () => { + const {result, router} = renderHookWithProviders(usePrimaryNavigation, { + additionalWrapper: PrimaryNavigationContextProvider, + initialRouterConfig: {location: {pathname: '/organizations/org-slug/issues/'}}, + }); + + act(() => result.current.setActiveGroup('explore')); + expect(result.current.activeGroup).toBe('explore'); + + act(() => router.navigate('/settings/org-slug/')); + expect(result.current.activeGroup).toBe('settings'); + + act(() => router.navigate('/organizations/org-slug/issues/')); + expect(result.current.activeGroup).toBe('issues'); + }); +}); diff --git a/static/app/views/navigation/primaryNavigationContext.tsx b/static/app/views/navigation/primaryNavigationContext.tsx index e3ccb10b0494..8dd7810fa96d 100644 --- a/static/app/views/navigation/primaryNavigationContext.tsx +++ b/static/app/views/navigation/primaryNavigationContext.tsx @@ -1,4 +1,4 @@ -import {createContext, useContext, useMemo, useState} from 'react'; +import {createContext, useCallback, useContext, useMemo, useState} from 'react'; import {useTheme} from '@emotion/react'; import * as Sentry from '@sentry/react'; @@ -70,21 +70,43 @@ interface PrimaryNavigationContextProviderProps { export function PrimaryNavigationContextProvider( props: PrimaryNavigationContextProviderProps ) { - const [activeGroupOverride, setActiveGroup] = useState(null); + const [activeGroupOverride, setActiveGroupOverride] = useState<{ + group: NavigationGroup; + routeGroup: NavigationGroup; + } | null>(null); const theme = useTheme(); const isMobile = useMedia(`(width < ${theme.breakpoints.md})`); const hasHover = useMedia('(hover: hover)'); const activeRouteGroup = useActiveNavigationGroup(); + // An override only lasts for the route group it was set under. Otherwise it + // sticks after navigating elsewhere (e.g. tapping Explore on mobile, then + // opening Settings from the org menu), leaving a stale secondary navigation. + // Clear it rather than ignore it, so it cannot come back when the user + // returns to the route group it was set under. + if (activeGroupOverride && activeGroupOverride.routeGroup !== activeRouteGroup) { + setActiveGroupOverride(null); + } + const activeGroupOverrideForRoute = + activeGroupOverride?.routeGroup === activeRouteGroup + ? activeGroupOverride.group + : null; + + const setActiveGroup = useCallback( + (group: NavigationGroup | null) => + setActiveGroupOverride(group ? {group, routeGroup: activeRouteGroup} : null), + [activeRouteGroup] + ); + const value = useMemo( () => ({ layout: isMobile ? ('mobile' as const) : ('sidebar' as const), features: {hover: hasHover}, - activeGroup: activeGroupOverride ?? activeRouteGroup, + activeGroup: activeGroupOverrideForRoute ?? activeRouteGroup, setActiveGroup, }), - [isMobile, hasHover, activeGroupOverride, activeRouteGroup] + [isMobile, hasHover, activeGroupOverrideForRoute, activeRouteGroup, setActiveGroup] ); return ( diff --git a/static/app/views/navigation/secondary/components.tsx b/static/app/views/navigation/secondary/components.tsx index 9d4d53ccd882..1948793c270f 100644 --- a/static/app/views/navigation/secondary/components.tsx +++ b/static/app/views/navigation/secondary/components.tsx @@ -287,46 +287,62 @@ interface SecondaryNavigationHeaderProps { function SecondaryNavigationHeader(props: SecondaryNavigationHeaderProps) { const {layout} = usePrimaryNavigation(); - const {view, setView} = useSecondaryNavigation(); - const isCollapsed = view !== 'expanded'; const isMobile = layout === 'mobile'; return ( + {props.children} + + + ); +} + +function SecondaryNavigationHeaderTitle({children}: {children: ReactNode}) { + const {layout} = usePrimaryNavigation(); + + return ( + - {props.children} + {children} -
- {isMobile ? ( -
- + + ); +} + +function SecondaryNavigationToggle() { + const {layout} = usePrimaryNavigation(); + const {view, setView} = useSecondaryNavigation(); + const isCollapsed = view !== 'expanded'; + + if (layout === 'mobile') { + return ( +