Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stacked Nav #77665

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
ef44e09
feat(nav): new nav components
natemoo-re Sep 13, 2024
50d85b9
feat(nav): update app layout
natemoo-re Sep 13, 2024
9cc12de
fix(nav): aria-current logic
natemoo-re Sep 13, 2024
cb8aee3
fix(nav): remove AnimatePresence
natemoo-re Sep 13, 2024
2edb4b2
feat(issues): add issue group consts
natemoo-re Sep 17, 2024
f4ba3f2
refactor(nav): support feature flagged nav items
natemoo-re Sep 17, 2024
78ec5f7
fix(nav): adjust insights display
natemoo-re Sep 17, 2024
d62ea20
wip(nav): mobile layout
natemoo-re Sep 18, 2024
a90be72
feat(nav): add top-level mobile menu
natemoo-re Sep 19, 2024
28e9d8f
fix(nav): adjust mobile layout
natemoo-re Sep 19, 2024
4de87d4
fix(nav): issue group highlighting
natemoo-re Sep 19, 2024
3d3411a
temp(nav): hardcode feature flag
natemoo-re Sep 20, 2024
33d31ad
fix(nav): improve issue filter ux
natemoo-re Sep 20, 2024
4c54072
test(nav): add basic test
natemoo-re Sep 20, 2024
dbfb818
fix(nav): improve accessibility
natemoo-re Sep 20, 2024
2316ab7
fix(nav): revert hardcoded flag
natemoo-re Sep 20, 2024
a434858
tmp(nav): remove non-link footer items
natemoo-re Sep 20, 2024
5b8e9f8
chore(nav): add braces to if statement
natemoo-re Sep 20, 2024
42dbd21
test(nav): fix test assertion
natemoo-re Sep 20, 2024
278745c
refactor(nav): update util naming
natemoo-re Sep 23, 2024
e25d831
refactor(nav): prefer pure function for location descriptor util
natemoo-re Sep 23, 2024
6b01eb3
Update static/app/types/hooks.tsx
natemoo-re Sep 23, 2024
9d4c362
refactor(hooks): update type name
natemoo-re Sep 23, 2024
b6aac27
Update static/app/components/nav/config.tsx
natemoo-re Sep 23, 2024
6dfe72a
Update static/app/components/nav/index.spec.tsx
natemoo-re Sep 23, 2024
a532d27
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Sep 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 191 additions & 0 deletions static/app/components/nav/config.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
import {useMemo} from 'react';

import type {NavItemRaw, SidebarItem, SubmenuItem} from 'sentry/components/nav/utils';

Check failure on line 3 in static/app/components/nav/config.tsx

View workflow job for this annotation

GitHub Actions / pre-commit lint

'SubmenuItem' is defined but never used. Allowed unused vars must match /^_/u

Check failure on line 3 in static/app/components/nav/config.tsx

View workflow job for this annotation

GitHub Actions / self-hosted

'SubmenuItem' is declared but never used.
import {
getNavigationItemStatus,
NAV_DIVIDER,
resolveSidebarItem,
splitAtDivider,
} from 'sentry/components/nav/utils';
import {
IconDashboard,
IconGraph,
IconIssues,
IconLightning,
IconProject,
IconSearch,
IconSettings,
IconSiren,
} from 'sentry/icons';
import {t} from 'sentry/locale';
import {getDiscoverLandingUrl} from 'sentry/utils/discover/urls';
import {useLocation} from 'sentry/utils/useLocation';
import useOrganization from 'sentry/utils/useOrganization';
import {useModuleURLBuilder} from 'sentry/views/insights/common/utils/useModuleURL';
import {MODULE_SIDEBAR_TITLE} from 'sentry/views/insights/http/settings';
import {MODULE_TITLES} from 'sentry/views/insights/settings';
import {getSearchForIssueGroup, IssueGroup} from 'sentry/views/issueList/utils';

export interface NavItemsResult {
primary: {
body: ReadonlyArray<SidebarItem>;
footer: ReadonlyArray<SidebarItem>;
};
secondary: {
body: ReadonlyArray<SidebarItem>;
footer: ReadonlyArray<SidebarItem>;
};
}

export function useNavItems(): NavItemsResult {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The useNavItems hook acts as a single source of truth for the entire nav, including nested navigation and feature flag checks.

const organization = useOrganization();
const location = useLocation();
const moduleURLBuilder = useModuleURLBuilder(true);
const prefix = `organizations/${organization.slug}`;

const items = useMemo<NavItemRaw[]>(
() => [
{
label: t('Issues'),
icon: <IconIssues />,
submenu: [
{
label: t('All'),
to: `/${prefix}/issues/?query=is:unresolved`,
},
{
label: t('Error & Outage'),
to: `/${prefix}/issues/${getSearchForIssueGroup(IssueGroup.ERROR_OUTAGE)}`,
},
{
label: t('Trend'),
to: `/${prefix}/issues/${getSearchForIssueGroup(IssueGroup.TREND)}`,
},
{
label: t('Craftsmanship'),
to: `/${prefix}/issues/${getSearchForIssueGroup(IssueGroup.CRAFTSMANSHIP)}`,
},
{
label: t('Security'),
to: `/${prefix}/issues/${getSearchForIssueGroup(IssueGroup.SECURITY)}`,
},
{label: t('Feedback'), to: `/${prefix}/feedback/`},
],
},
{label: t('Projects'), to: `/${prefix}/projects/`, icon: <IconProject />},
{
label: t('Explore'),
icon: <IconSearch />,
submenu: [
{
label: t('Traces'),
to: `/traces/`,
check: {features: 'performance-trace-explorer'},
},
{
label: t('Metrics'),
to: `/${prefix}/metrics/`,
check: {features: 'custom-metrics'},
},
{
label: t('Profiles'),
to: `/${prefix}/profiling/`,
check: {features: 'profiling', hook: 'profiling-sidebar-item'},
},
{
label: t('Replays'),
to: `/${prefix}/replays/`,
check: {features: 'session-replay-ui', hook: 'replay-sidebar-item'},
},
{
label: t('Discover'),
to: getDiscoverLandingUrl(organization),
check: {features: 'discover-basic', hook: 'discover2-sidebar-item'},
},
{label: t('Releases'), to: `/${prefix}/releases/`},
{label: t('Crons'), to: `/${prefix}/crons/`},
],
},
{
label: t('Insights'),
icon: <IconGraph />,
check: {features: 'insights-entry-points'},
submenu: [
{label: MODULE_SIDEBAR_TITLE, to: `/${prefix}/${moduleURLBuilder('http')}/`},
{label: MODULE_TITLES.db, to: `/${prefix}/${moduleURLBuilder('db')}/`},
{
label: MODULE_TITLES.resource,
to: `/${prefix}/${moduleURLBuilder('resource')}/`,
},
{
label: MODULE_TITLES.app_start,
to: `/${prefix}/${moduleURLBuilder('app_start')}/`,
},
{
label: MODULE_TITLES['mobile-screens'],
to: `/${prefix}/${moduleURLBuilder('mobile-screens')}/`,
check: {features: 'insights-mobile-screens-module'},
},
{label: MODULE_TITLES.vital, to: `/${prefix}/${moduleURLBuilder('vital')}/`},
{label: MODULE_TITLES.cache, to: `/${prefix}/${moduleURLBuilder('cache')}/`},
{label: MODULE_TITLES.queue, to: `/${prefix}/${moduleURLBuilder('queue')}/`},
{
label: MODULE_TITLES.ai,
to: `/${prefix}/${moduleURLBuilder('ai')}/`,
check: {features: 'insights-entry-points'},
},
],
},
{
label: t('Perf.'),
to: '/performance/',
icon: <IconLightning />,
check: {features: 'performance-view', hook: 'performance-sidebar-item'},
},
{
label: t('Boards'),
to: '/dashboards/',
icon: <IconDashboard />,
check: {
features: ['discover', 'discover-query', 'dashboards-basic', 'dashboards-edit'],
hook: 'dashboards-sidebar-item',
},
},
{label: t('Alerts'), to: `/${prefix}/alerts/rules/`, icon: <IconSiren />},
NAV_DIVIDER,
// {label: t('Help'), to: '', icon: <IconQuestion />},
// {label: t('New'), to: '', icon: <IconBroadcast />},
// {label: t('Stats'), to: '', icon: <IconStats />},
Comment on lines +156 to +158
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// {label: t('Help'), to: '', icon: <IconQuestion />},
// {label: t('New'), to: '', icon: <IconBroadcast />},
// {label: t('Stats'), to: '', icon: <IconStats />},

{
label: t('Settings'),
to: `/settings/${organization.slug}/`,
icon: <IconSettings />,
},
],
[organization, moduleURLBuilder, prefix]
);

const formatted = useMemo(() => formatNavItems({location}, items), [location, items]);
return formatted;
}

function formatNavItems(
context: {location: ReturnType<typeof useLocation>},
items: NavItemRaw[]
): NavItemsResult {
const sidebar = items
.filter(item => !!item)
.map(item => (typeof item === 'object' ? resolveSidebarItem(item) : item));
const primary = splitAtDivider(sidebar);
const {submenu = []} = primary.body.find(
item => getNavigationItemStatus(item, context.location) !== 'inactive'
) ?? {
submenu: [],
};
const secondary = splitAtDivider(submenu);

return {
primary,
secondary,

Check failure on line 189 in static/app/components/nav/config.tsx

View workflow job for this annotation

GitHub Actions / self-hosted

Type '{ body: SubmenuItem[]; footer: SubmenuItem[]; }' is not assignable to type '{ body: readonly SidebarItem[]; footer: readonly SidebarItem[]; }'.
};
}
104 changes: 104 additions & 0 deletions static/app/components/nav/index.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import {LocationFixture} from 'sentry-fixture/locationFixture';
import {OrganizationFixture} from 'sentry-fixture/organization';
import {UserFixture} from 'sentry-fixture/user';

import {render, screen} from 'sentry-test/reactTestingLibrary';

import Nav from 'sentry/components/nav';
import ConfigStore from 'sentry/stores/configStore';
import type {Organization} from 'sentry/types/organization';
import {useLocation} from 'sentry/utils/useLocation';

jest.mock('sentry/actionCreators/account');
jest.mock('sentry/utils/useServiceIncidents');
jest.mock('sentry/utils/useLocation');

const mockUseLocation = jest.mocked(useLocation);

const ALL_AVAILABLE_FEATURES = [
'insights-entry-points',
'discover',
'discover-basic',
'discover-query',
'dashboards-basic',
'dashboards-edit',
'custom-metrics',
'user-feedback-ui',
'session-replay-ui',
'performance-view',
'performance-trace-explorer',
'starfish-mobile-ui-module',
'profiling',
];

describe('Nav', function () {
const organization = OrganizationFixture();
const user = UserFixture();

const getElement = () => <Nav />;

const renderNav = ({organization: org}: {organization: Organization | null}) =>
render(getElement(), {organization: org});

const renderNavWithFeatures = (features: string[] = []) => {
return renderNav({
organization: {
...organization,
features: [...organization.features, ...features],
},
});
};

beforeEach(function () {
mockUseLocation.mockReturnValue(LocationFixture());
});

afterEach(function () {
mockUseLocation.mockReset();
});

it('renders', async function () {
renderNav({organization});
expect(
await screen.findByRole('navigation', {name: 'Primary Navigation'})
).toBeInTheDocument();
});

describe('sidebar links', () => {
beforeEach(function () {
ConfigStore.init();
ConfigStore.set('features', new Set([]));
ConfigStore.set('user', user);

mockUseLocation.mockReturnValue({...LocationFixture()});
});
afterEach(() => ConfigStore.reset());
it('renders navigation', function () {
renderNav({organization});

expect(
screen.getByRole('navigation', {name: 'Primary Navigation'})
).toBeInTheDocument();
});

it('renders all features', function () {
renderNavWithFeatures([...ALL_AVAILABLE_FEATURES]);

const links = screen.getAllByRole('link');
expect(links).toHaveLength(8);

[
'Issues',
'Projects',
'Explore',
'Insights',
'Perf.',
'Boards',
'Alerts',
'Settings',
].forEach((title, index) => {
expect(links[index]).toHaveAccessibleName(title);
});
});
});
});
74 changes: 74 additions & 0 deletions static/app/components/nav/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import styled from '@emotion/styled';

import OrganizationAvatar from 'sentry/components/avatar/organizationAvatar';
import {useNavItems} from 'sentry/components/nav/config';
import {Mobile} from 'sentry/components/nav/mobile';
import Sidebar from 'sentry/components/nav/sidebar';
import Submenu from 'sentry/components/nav/submenu';
import {useBreakpoints} from 'sentry/utils/metrics/useBreakpoints';
import useOrganization from 'sentry/utils/useOrganization';

function Nav() {
const organization = useOrganization();
const nav = useNavItems();
const screen = useBreakpoints();

if (!screen.medium) {
return (
<NavContainer>
<Mobile />
</NavContainer>
);
}

return (
<NavContainer>
<Sidebar role="navigation" aria-label="Primary Navigation">
<Sidebar.Header>
<OrganizationAvatar organization={organization} size={32} />
</Sidebar.Header>
<Sidebar.Body>
{nav.primary.body.map(item => (
<Sidebar.Item key={item.label} {...item} />
))}
</Sidebar.Body>
<Sidebar.Footer>
{nav.primary.footer.map(item => (
<Sidebar.Item key={item.label} {...item} />
))}
</Sidebar.Footer>
</Sidebar>
{nav.secondary.body.length > 0 && (
<Submenu role="navigation" aria-label="Secondary Navigation">
<Submenu.Body>
{nav.secondary.body.map(item => (
<Submenu.Item key={item.label} {...item} />
))}
</Submenu.Body>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to insert a <Sidebar.Divider/> or equiv inside rendering decl instead of handle it inside the config?

{nav.secondary.footer.length > 0 && (
<Submenu.Footer>
{nav.secondary.footer.map(item => (
<Submenu.Item key={item.label} {...item} />
))}
</Submenu.Footer>
)}
</Submenu>
)}
</NavContainer>
);
}

const NavContainer = styled('div')`
display: flex;
position: sticky;
top: 0;
z-index: ${p => p.theme.zIndex.sidebarPanel};

@media screen and (min-width: ${p => p.theme.breakpoints.medium}) {
bottom: 0;
height: 100vh;
height: 100dvh;
}
`;

export default Nav;
Loading
Loading