Sidebar design update #76868

natemoo-re · 2024-09-03T17:54:18Z

This PR updates the sidebar design across the Sentry frontend, behind the navigation-sidebar-v2 flag. This PR supercedes #76577.

TODO

handle alpha / beta badges
add subnav panel
regression tests
mobile (maintain previous UI)

Follow-up

overlays (what's new, onboarding) as modal dialogs
new mobile designs

codecov · 2024-09-03T18:00:33Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
7761	1	7760	0

View the top 1 failed tests by shortest run time

Sidebar Stacked navigation with navigation-sidebar-v2 flag does not render secondary subnav for projects Sidebar Stacked navigation with navigation-sidebar-v2 flag does not render secondary subnav for projects

Stack Traces | 0.06s run time

Error: expect(element).not.toBeInTheDocument()

expected document not to contain element, found &lt;div aria-label=&quot;Secondary Navigation&quot; class=&quot;css-bz8ur1-SidebarSecondaryWrapper e1pyhmmi0&quot; /&gt; instead
    at Object.&lt;anonymous&gt; (.../components/sidebar/index.spec.tsx:368:93)

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

natemoo-re · 2024-09-10T17:19:51Z

static/app/components/sidebar/index.tsx

 />
 );
+ if (hasNewNav && hasOrganization) {
+ issues = (
+ <SubnavMenu


Most of the functional markup changes are handled by a new SubnavMenu component, which projects children into the secondary sidebar panel as needed

natemoo-re · 2024-09-10T17:23:06Z

static/app/components/sidebar/sidebarItem.tsx

+ // issue subnav matches against issue groups in search params
+ if (location.pathname.startsWith('/issues/') && item?.to?.includes('/issues/')) {
+ const search = new URLSearchParams(location.search);
+ const itemSearch = new URLSearchParams(item.search);
+ const itemQuery = itemSearch.get('query');
+ const query = search.get('query');
+ if (item?.label === 'All') {
+ return !query && !itemQuery;
+ }
+ if (itemQuery && query) {
+ let match = false;
+ for (const key of itemQuery?.split(' ')) {
+ match = query.includes(key);
+ if (!match) break;
+ }
+ return match;
+ }
+ return !itemQuery;
+ }


This is new "active" matching logic for the Issues subnav which required special consideration of search params.

See also: #76868 (comment)

natemoo-re · 2024-09-10T17:25:15Z

static/app/components/sidebar/subnav.tsx

+ useLayoutEffect(() => {
+ if (!containerRef.current) return;
+ if (containerRef.current.children.length === 0) {
+ document.body.dataset.navLevel = '1';
+ } else {
+ document.body.dataset.navLevel = '2';
+ }
+ });


Not the cleanest pattern, but since the sidebar uses position: fixed, we need an effect that surfaces how many navigation levels are active so we can adjust the static app layout from inside the render tree.

heh
would there ever be a world where we have more layers than just 2?

Good question! I don't imagine there would be, so this could be treated as a boolean attribute or class name.

natemoo-re · 2024-09-10T17:26:20Z

static/app/utils/theme.tsx

+ v2_width: '80px',
+ v2_panelWidth: '160px',


Slight adjustments from the spec'd designs to accommodate font rendering differences

static/app/views/issueList/utils.tsx

nhsiehgit

buncha nit comments!

nhsiehgit · 2024-09-10T18:19:25Z

static/app/components/sidebar/index.tsx

- {issues}
- {projects}
- </SidebarSection>
+ <SidebarWrapper>


Ah i see
iiuc, we're using the SubnavContainer component here as the main panel? (then in the children determines, whether to render a secondary panel OR the accordion?)

Do we need this wrapper anymore then or can we move the styles here to the container/primary wrapper?

Also - wonder if we should rename the SubnavContainer to just be NavContainer 😅

nhsiehgit · 2024-09-10T18:21:24Z

static/app/components/sidebar/index.tsx

- )}
+ {hasOrganization && (
+ <SidebarSectionGroup hasNewNav={hasNewNav}>
+ {/* What are the onboarding sidebars? */}


oops - was this a comment i left?? 😅
can probably remove

nhsiehgit · 2024-09-10T18:22:32Z

static/app/components/sidebar/index.tsx

 const SidebarSectionGroup = styled('div')<{hasNewNav?: boolean}>`
 ${responsiveFlex};
 flex-shrink: 0; /* prevents shrinking on Safari */
- gap: 1px;
- ${p => p.hasNewNav && `align-items: center;`}
+ gap: 8px;


do we have any consts for this? maybe space(1) or something?

nhsiehgit · 2024-09-10T18:31:35Z

static/app/components/sidebar/sidebarItem.tsx

 exact?: boolean
 ): boolean {
+ // issue subnav matches against issue groups in search params
+ if (location.pathname.startsWith('/issues/') && item?.to?.includes('/issues/')) {


Will this only be relevant for issues subnav?
Will we ever need to implement logic for other routes?

wondering about scalability and how we might structure this or break this out to be more composable 🤷

nhsiehgit · 2024-09-10T18:33:49Z

static/app/components/sidebar/subnav.tsx

+ useLayoutEffect(() => {
+ if (!containerRef.current) return;
+ if (containerRef.current.children.length === 0) {
+ document.body.dataset.navLevel = '1';
+ } else {
+ document.body.dataset.navLevel = '2';
+ }
+ });


heh
would there ever be a world where we have more layers than just 2?

evanpurkhiser · 2024-09-10T21:52:03Z

static/app/components/sidebar/index.spec.tsx

+ organization: {...organization, features: ['navigation-sidebar-v2']},
+ });
+
+ expect(await screen.findByText('Issues')).toBeInTheDocument();


Should it be findByRole('link', {name: 'Issues'})? Or is it a button findByRole('button', {name: 'Issues'})

codecov · 2024-09-10T22:08:17Z

Bundle Report

Changes will increase total bundle size by 35.6kB (0.12%) ⬆️. This is within the configured threshold ✅

Detailed changes

Bundle name	Size	Change
app-webpack-bundle-array-push	30.28MB	35.6kB ⬆️

ryan953 · 2024-09-11T16:50:32Z

static/app/views/replays/detail/layout/utils.tsx

The profiling flamechart might do something similar to this. I remember they had a full-screen view first then replay did something similar afterwards.
I'm not confident that old code survived or not, but that'd be a view to double-check.

natemoo-re · 2024-09-17T22:34:19Z

Closing in favor of #77665. The new PR should be much easier to review, since it introduces entirely new components for the navigation.

github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Sep 3, 2024

natemoo-re mentioned this pull request Sep 3, 2024

WIP: Nhsieh/sidenavbarv2 #76577

Closed

vercel bot deployed to Preview September 3, 2024 20:12 View deployment

natemoo-re force-pushed the natemoo-re/sidebar-v2 branch from 3d6f516 to 3234afd Compare September 9, 2024 20:56

vercel bot deployed to Preview September 9, 2024 21:04 View deployment

natemoo-re force-pushed the natemoo-re/sidebar-v2 branch from 17a232c to c9f785e Compare September 9, 2024 21:34

vercel bot deployed to Preview September 9, 2024 21:42 View deployment

vercel bot deployed to Preview September 9, 2024 22:02 View deployment

natemoo-re force-pushed the natemoo-re/sidebar-v2 branch from 4dbe461 to 09276af Compare September 9, 2024 22:05

vercel bot deployed to Preview September 9, 2024 22:08 View deployment

natemoo-re force-pushed the natemoo-re/sidebar-v2 branch from 1eea734 to cdbe206 Compare September 10, 2024 16:11

vercel bot deployed to Preview September 10, 2024 16:18 View deployment

natemoo-re added 15 commits September 10, 2024 12:15

style(sidebar): update sidebar css for v2 design

4927def

fix(sidebar): restore feature flag

1ff19ac

wip(sidebar): update explore + insights nav

b8edcd4

wip(sidebar): add subnav panel

2328e88

fix(sidebar): add subnav toggle logic

7f8f415

fix(sidebar): adjust hover styles

ce0cc26

feat(sidebar): add issue category nav items

d2e58fe

fix(sidebar): remove hardcoded value

a187ba9

fix(sidebar): remove incorrect state layer styles

fbe94e0

test: adjust sidebar link test

84be595

fix(sidebar): adjust panel width

190ccac

refactor(sidebar): prefer IssueGroup name

4a17b7b

fix(sidebar): support nav highlighting for issue groups

b35520b

fix(sidebar): revert hardcoded value

b27f993

test(sidebar): group new nav tests

8553647

natemoo-re force-pushed the natemoo-re/sidebar-v2 branch from 2b096c5 to 8553647 Compare September 10, 2024 17:15

vercel bot deployed to Preview September 10, 2024 17:22 View deployment

natemoo-re commented Sep 10, 2024

View reviewed changes

nhsiehgit approved these changes Sep 10, 2024

View reviewed changes

evanpurkhiser reviewed Sep 10, 2024

View reviewed changes

natemoo-re added 3 commits September 10, 2024 17:01

test(sidebar): improve v2 tests

d2cb1a5

fix(sidebar): do not change markup without flag

9a2af47

chore(sidebar): remove comment

ac9b6c8

vercel bot deployed to Preview September 10, 2024 22:05 View deployment

vercel bot deployed to Preview September 11, 2024 15:01 View deployment

ryan953 reviewed Sep 11, 2024

View reviewed changes

natemoo-re mentioned this pull request Sep 17, 2024

Stacked Nav #77665

Open

2 tasks

natemoo-re closed this Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sidebar design update #76868

Sidebar design update #76868

natemoo-re commented Sep 3, 2024 •

edited

Loading

codecov bot commented Sep 3, 2024 •

edited

Loading

natemoo-re Sep 10, 2024

natemoo-re Sep 10, 2024 •

edited

Loading

natemoo-re Sep 10, 2024

nhsiehgit Sep 10, 2024

natemoo-re Sep 10, 2024

natemoo-re Sep 10, 2024

nhsiehgit left a comment

nhsiehgit Sep 10, 2024

nhsiehgit Sep 10, 2024

nhsiehgit Sep 10, 2024

nhsiehgit Sep 10, 2024

nhsiehgit Sep 10, 2024

evanpurkhiser Sep 10, 2024

codecov bot commented Sep 10, 2024 •

edited

Loading

ryan953 Sep 11, 2024

natemoo-re commented Sep 17, 2024

Sidebar design update #76868

Sidebar design update #76868

Conversation

natemoo-re commented Sep 3, 2024 • edited Loading

TODO

Follow-up

codecov bot commented Sep 3, 2024 • edited Loading

❌ 1 Tests Failed:

Choose a reason for hiding this comment

natemoo-re Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nhsiehgit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 10, 2024 • edited Loading

Bundle Report

Choose a reason for hiding this comment

natemoo-re commented Sep 17, 2024

natemoo-re commented Sep 3, 2024 •

edited

Loading

codecov bot commented Sep 3, 2024 •

edited

Loading

natemoo-re Sep 10, 2024 •

edited

Loading

codecov bot commented Sep 10, 2024 •

edited

Loading