Skip to content

Commit e827137

Browse files
authored
Improves sidebar content robustness (#2883)
* Improves sidebar content robustness Adds error handling and graceful loading states to the tutorial view sidebar component. This prevents crashes and improves the user experience when data is loading or missing, and during rapid navigation changes. It achieves this by wrapping the component in an ErrorBoundary and ensuring the CollectionProgress component always renders its container, which prevents layout shifts. * Simplifies collection progress handling comments Refactors the collection progress component to streamline data fetching and rendering logic. Removes an unnecessary condition that always rendered the container, simplifying the component's structure.
1 parent 8a10a7d commit e827137

File tree

2 files changed

+208
-16
lines changed

2 files changed

+208
-16
lines changed
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
import { vi } from 'vitest'
2+
3+
// Mock hooks to avoid provider/context requirements and network
4+
vi.mock('hooks/progress', () => ({
5+
useCollectionProgress: () => ({ data: {}, isLoading: false }),
6+
useTutorialProgress: () => ({ tutorialProgressStatus: 'not-started' }),
7+
}))
8+
9+
// Mock child components to simple renderers
10+
vi.mock('components/collection-progress-group', () => ({
11+
CollectionProgressStatusSection: ({ completedTutorialCount, tutorialCount, isInProgress }) => (
12+
<div data-testid="collection-progress">
13+
Progress: {completedTutorialCount}/{tutorialCount} {isInProgress ? '(in progress)' : ''}
14+
</div>
15+
),
16+
parseCollectionProgress: () => ({ completedTutorialCount: 0, tutorialCount: 0, isInProgress: false }),
17+
}))
18+
19+
vi.mock('components/tutorial-progress-icon', () => ({
20+
default: () => <span data-testid="tutorial-progress-icon" />,
21+
}))
22+
23+
vi.mock('components/tutorials-sidebar', () => ({
24+
SectionList: ({ children }) => <div data-testid="section-list">{children}</div>,
25+
}))
26+
27+
vi.mock('components/sidebar/components', () => ({
28+
SidebarNavMenuItem: ({ item }) => (
29+
<a href={item.href} data-active={item.isActive ? 'true' : 'false'}>
30+
{item.title}
31+
</a>
32+
),
33+
}))
34+
35+
import { render, screen } from '@testing-library/react'
36+
import { describe, it, expect } from 'vitest'
37+
import { ErrorBoundary } from 'components/error-boundary'
38+
import TutorialViewSidebarContent from '../index'
39+
import type { Collection } from 'lib/learn-client/types'
40+
import type { TutorialListItemProps } from 'components/tutorials-sidebar/types'
41+
import userEvent from '@testing-library/user-event'
42+
43+
describe('TutorialViewSidebarContent', () => {
44+
it('renders with data and correct href', () => {
45+
const mockCollection = {
46+
id: 'collection-id',
47+
slug: 'collection-slug',
48+
tutorials: [{ id: 'tutorial-id', title: 'Tutorial Title' }],
49+
} as unknown as Collection
50+
51+
const mockItems: TutorialListItemProps[] = [
52+
{
53+
text: 'Tutorial Title',
54+
href: '/tutorial',
55+
isActive: false,
56+
tutorialId: 'tutorial-id',
57+
collectionId: 'collection-id',
58+
},
59+
]
60+
61+
render(<TutorialViewSidebarContent collection={mockCollection} items={mockItems} />)
62+
63+
expect(screen.getByText('Tutorial Title')).toBeInTheDocument()
64+
const link = screen.getByText('Tutorial Title').closest('a')
65+
expect(link).toHaveAttribute('href', '/tutorial')
66+
})
67+
68+
it('handles loading state gracefully (no crash, container renders)', () => {
69+
const mockCollection = {} as unknown as Collection // Simulate loading/missing data
70+
const mockItems: TutorialListItemProps[] = []
71+
72+
render(
73+
<ErrorBoundary>
74+
<TutorialViewSidebarContent collection={mockCollection} items={mockItems} />
75+
</ErrorBoundary>
76+
)
77+
78+
expect(screen.queryByText('Tutorial Title')).not.toBeInTheDocument()
79+
expect(screen.getByTestId('section-list')).toBeInTheDocument()
80+
})
81+
82+
it('handles rapid navigation (re-render) without errors', async () => {
83+
const mockCollection1 = {
84+
id: 'collection-1',
85+
slug: 'collection-1-slug',
86+
tutorials: [{ id: 'tutorial-1', title: 'Tutorial 1' }],
87+
} as unknown as Collection
88+
89+
const mockCollection2 = {
90+
id: 'collection-2',
91+
slug: 'collection-2-slug',
92+
tutorials: [{ id: 'tutorial-2', title: 'Tutorial 2' }],
93+
} as unknown as Collection
94+
95+
const mockItems1: TutorialListItemProps[] = [
96+
{
97+
text: 'Tutorial 1',
98+
href: '/tutorial-1',
99+
isActive: false,
100+
tutorialId: 'tutorial-1',
101+
collectionId: 'collection-1',
102+
},
103+
]
104+
105+
const mockItems2: TutorialListItemProps[] = [
106+
{
107+
text: 'Tutorial 2',
108+
href: '/tutorial-2',
109+
isActive: false,
110+
tutorialId: 'tutorial-2',
111+
collectionId: 'collection-2',
112+
},
113+
]
114+
115+
const { rerender } = render(
116+
<ErrorBoundary>
117+
<TutorialViewSidebarContent collection={mockCollection1} items={mockItems1} />
118+
</ErrorBoundary>
119+
)
120+
121+
expect(screen.getByText('Tutorial 1')).toBeInTheDocument()
122+
123+
rerender(
124+
<ErrorBoundary>
125+
<TutorialViewSidebarContent collection={mockCollection2} items={mockItems2} />
126+
</ErrorBoundary>
127+
)
128+
129+
expect(screen.getByText('Tutorial 2')).toBeInTheDocument()
130+
expect(screen.queryByText('Tutorial 1')).not.toBeInTheDocument()
131+
})
132+
133+
it('navigates correctly when clicking a sidebar item (no error)', async () => {
134+
const mockCollection = {
135+
id: 'collection-id',
136+
slug: 'collection-slug',
137+
tutorials: [{ id: 'tutorial-id', title: 'Tutorial Title' }],
138+
} as unknown as Collection
139+
140+
const mockItems: TutorialListItemProps[] = [
141+
{
142+
text: 'Tutorial Title',
143+
href: '/tutorial',
144+
isActive: false,
145+
tutorialId: 'tutorial-id',
146+
collectionId: 'collection-id',
147+
},
148+
]
149+
150+
render(<TutorialViewSidebarContent collection={mockCollection} items={mockItems} />)
151+
152+
const linkEl = screen.getByText('Tutorial Title')
153+
await userEvent.click(linkEl)
154+
155+
expect(screen.getByText('Tutorial Title')).toBeInTheDocument()
156+
expect(screen.queryByText(/Something went wrong/)).not.toBeInTheDocument()
157+
const link = linkEl.closest('a')
158+
expect(link).toHaveAttribute('href', '/tutorial')
159+
})
160+
})

src/components/tutorials-sidebar/components/tutorial-view-sidebar-content/index.tsx

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,13 @@ import {
1515
import TutorialProgressIcon from 'components/tutorial-progress-icon'
1616
import { SidebarNavMenuItem } from 'components/sidebar/components'
1717
import { SectionList } from 'components/tutorials-sidebar'
18+
import { ErrorBoundary } from 'components/error-boundary'
1819
// Types
1920
import { TutorialListItemProps } from 'components/tutorials-sidebar/types'
2021
// Styles
2122
import s from './tutorial-view-sidebar-content.module.css'
2223
import { Collection } from 'lib/learn-client/types'
24+
import React from 'react'
2325

2426
/**
2527
* Renders sidebar content for tutorial views.
@@ -56,30 +58,44 @@ function TutorialViewSidebarContent({
5658
* Displays collection progress status.
5759
*/
5860
function CollectionProgress({ collection }: { collection: Collection }) {
59-
const { id, slug, tutorials } = collection
61+
const { id, slug, tutorials } = collection ?? {}
6062

6163
/**
6264
* Get collection progress, which affects the
6365
* CTA bar we display for the collection.
6466
*/
65-
const { data: progressData } = useCollectionProgress({ collectionId: id })
67+
const { data: progressData, isLoading } = useCollectionProgress({
68+
collectionId: id,
69+
})
6670

6771
/**
6872
* Parse the progress-related information we need from the progress records,
6973
* current collection slug, and list of tutorials in this collection.
7074
*/
71-
const { completedTutorialCount, tutorialCount, isInProgress } = useMemo(
72-
() => parseCollectionProgress(progressData, tutorials.length, { id, slug }),
73-
[progressData, tutorials, id, slug]
74-
)
75+
const { completedTutorialCount, tutorialCount, isInProgress } =
76+
useMemo(() => {
77+
if (!collection || !id || !tutorials) {
78+
return {
79+
completedTutorialCount: 0,
80+
tutorialCount: 0,
81+
isInProgress: false,
82+
}
83+
}
84+
return parseCollectionProgress(progressData, tutorials.length, {
85+
id,
86+
slug,
87+
})
88+
}, [progressData, tutorials, id, slug, collection])
7589

7690
return (
7791
<div className={s.collectionProgressContainer}>
78-
<CollectionProgressStatusSection
79-
completedTutorialCount={completedTutorialCount}
80-
tutorialCount={tutorialCount}
81-
isInProgress={isInProgress}
82-
/>
92+
{!isLoading && collection && id && tutorials && (
93+
<CollectionProgressStatusSection
94+
completedTutorialCount={completedTutorialCount}
95+
tutorialCount={tutorialCount}
96+
isInProgress={isInProgress}
97+
/>
98+
)}
8399
</div>
84100
)
85101
}
@@ -96,19 +112,35 @@ function TutorialListItem({
96112
collectionId,
97113
}: TutorialListItemProps) {
98114
/**
99-
* Query for progress, and display the appropriate status icon
115+
* Query for progress if we have valid IDs
100116
*/
101117
const { tutorialProgressStatus } = useTutorialProgress({
102118
tutorialId,
103119
collectionId,
104120
})
105-
const trailingIcon = (
106-
<TutorialProgressIcon status={tutorialProgressStatus} isActive={isActive} />
107-
)
121+
122+
/**
123+
* Only show the progress icon if we have valid IDs
124+
*/
125+
const trailingIcon =
126+
tutorialId && collectionId ? (
127+
<TutorialProgressIcon
128+
status={tutorialProgressStatus}
129+
isActive={isActive}
130+
/>
131+
) : null
108132

109133
return (
110134
<SidebarNavMenuItem item={{ isActive, title: text, href, trailingIcon }} />
111135
)
112136
}
113137

114-
export default TutorialViewSidebarContent
138+
function TutorialViewSidebarContentWithBoundary(props) {
139+
return (
140+
<ErrorBoundary>
141+
<TutorialViewSidebarContent {...props} />
142+
</ErrorBoundary>
143+
)
144+
}
145+
146+
export default TutorialViewSidebarContentWithBoundary

0 commit comments

Comments
 (0)