Skip to content

Commit ab0f5e7

Browse files
romanlutzCopilot
andauthored
FEAT Make onboarding tour opt-in via a top-bar button (#2069)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent e8ae6f7 commit ab0f5e7

8 files changed

Lines changed: 74 additions & 53 deletions

File tree

frontend/src/App.test.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ jest.mock("react-joyride", () => ({
2929
jest.mock("./hooks/useTour", () => ({
3030
useTour: () => ({
3131
startTour: jest.fn(),
32-
hasCompletedTour: true,
3332
tourProps: {
3433
steps: [],
3534
run: false,

frontend/src/App.tsx

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -335,17 +335,10 @@ function App() {
335335
/>
336336
)
337337

338-
// Onboarding tour — pass handleNavigate so the tour can switch views between steps
338+
// Onboarding tour — pass handleNavigate so the tour can switch views between steps.
339+
// The tour does not auto-start; users launch it from the "Take a tour" button in the top bar.
339340
const { resolved } = useTheme()
340-
const { startTour, hasCompletedTour, tourProps } = useTour(handleNavigate, resolved === 'dark', currentView)
341-
342-
// Auto-start the tour on first visit
343-
useEffect(() => {
344-
if (!hasCompletedTour) {
345-
startTour()
346-
}
347-
// eslint-disable-next-line react-hooks/exhaustive-deps
348-
}, [])
341+
const { startTour, tourProps } = useTour(handleNavigate, resolved === 'dark', currentView)
349342

350343
return (
351344
<ErrorBoundary>

frontend/src/components/Layout/MainLayout.styles.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ export const useMainLayoutStyles = makeStyles({
3232
color: tokens.colorNeutralForeground3,
3333
marginLeft: tokens.spacingHorizontalXS,
3434
},
35+
spacer: {
36+
flex: 1,
37+
},
3538
contentArea: {
3639
display: 'flex',
3740
flex: 1,

frontend/src/components/Layout/MainLayout.test.tsx

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55

66
import { render, screen, waitFor } from "@testing-library/react";
7+
import userEvent from "@testing-library/user-event";
78
import { FluentProvider, webLightTheme } from "@fluentui/react-components";
89
import MainLayout from "./MainLayout";
910

@@ -138,4 +139,55 @@ describe("MainLayout", () => {
138139
expect(mockedVersionApi.getVersion).toHaveBeenCalled();
139140
});
140141
});
142+
143+
it("renders a 'Take a tour' button in the top bar when onStartTour is provided", async () => {
144+
mockedVersionApi.getVersion.mockResolvedValue({ version: "1.0.0" });
145+
146+
renderWithProvider(
147+
<MainLayout {...defaultProps} onStartTour={jest.fn()}>
148+
<div>Content</div>
149+
</MainLayout>
150+
);
151+
152+
expect(screen.getByRole("button", { name: /take a tour/i })).toBeInTheDocument();
153+
154+
await waitFor(() => {
155+
expect(mockedVersionApi.getVersion).toHaveBeenCalled();
156+
});
157+
});
158+
159+
it("calls onStartTour when the 'Take a tour' button is clicked", async () => {
160+
mockedVersionApi.getVersion.mockResolvedValue({ version: "1.0.0" });
161+
const onStartTour = jest.fn();
162+
const user = userEvent.setup();
163+
164+
renderWithProvider(
165+
<MainLayout {...defaultProps} onStartTour={onStartTour}>
166+
<div>Content</div>
167+
</MainLayout>
168+
);
169+
170+
await user.click(screen.getByRole("button", { name: /take a tour/i }));
171+
expect(onStartTour).toHaveBeenCalledTimes(1);
172+
173+
await waitFor(() => {
174+
expect(mockedVersionApi.getVersion).toHaveBeenCalled();
175+
});
176+
});
177+
178+
it("does not render the 'Take a tour' button when onStartTour is not provided", async () => {
179+
mockedVersionApi.getVersion.mockResolvedValue({ version: "1.0.0" });
180+
181+
renderWithProvider(
182+
<MainLayout {...defaultProps}>
183+
<div>Content</div>
184+
</MainLayout>
185+
);
186+
187+
expect(screen.queryByRole("button", { name: /take a tour/i })).not.toBeInTheDocument();
188+
189+
await waitFor(() => {
190+
expect(mockedVersionApi.getVersion).toHaveBeenCalled();
191+
});
192+
});
141193
});

frontend/src/components/Layout/MainLayout.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { useEffect, useState } from 'react'
22
import {
3+
Button,
34
Text,
45
Tooltip,
56
} from '@fluentui/react-components'
7+
import { QuestionCircleRegular } from '@fluentui/react-icons'
68
import { versionApi } from '../../services/api'
79
import Navigation, { type ViewName } from '../Sidebar/Navigation'
810
import { UserAccountButton } from '../UserAccountButton'
@@ -48,6 +50,17 @@ export default function MainLayout({
4850
</Tooltip>
4951
<Text className={styles.title}>Co-PyRIT</Text>
5052
<Text className={styles.subtitle}>Python Risk Identification Tool</Text>
53+
<div className={styles.spacer} />
54+
{onStartTour && (
55+
<Button
56+
appearance="subtle"
57+
icon={<QuestionCircleRegular />}
58+
onClick={onStartTour}
59+
data-testid="start-tour"
60+
>
61+
Take a tour
62+
</Button>
63+
)}
5164
<UserAccountButton />
5265
</div>
5366
<div className={styles.contentArea}>
@@ -56,7 +69,6 @@ export default function MainLayout({
5669
currentView={currentView}
5770
onNavigate={onNavigate}
5871
onOpenFeedback={onOpenFeedback}
59-
onStartTour={onStartTour}
6072
/>
6173
</aside>
6274
<main className={styles.main}>{children}</main>

frontend/src/components/Sidebar/Navigation.tsx

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import type { MenuCheckedValueChangeData, MenuCheckedValueChangeEvent } from '@f
1010
import {
1111
ChatRegular,
1212
HomeRegular,
13-
QuestionCircleRegular,
1413
SettingsRegular,
1514
HistoryRegular,
1615
PersonFeedbackRegular,
@@ -26,7 +25,6 @@ export type ViewName = 'home' | 'chat' | 'history' | 'config'
2625
interface NavigationProps {
2726
currentView: ViewName
2827
onNavigate: (view: ViewName) => void
29-
onStartTour?: () => void
3028
onOpenFeedback: () => void
3129
}
3230

@@ -39,7 +37,7 @@ const THEME_LABELS: Record<ThemeMode, string> = {
3937
}
4038

4139

42-
export default function Navigation({ currentView, onNavigate, onStartTour, onOpenFeedback }: NavigationProps) {
40+
export default function Navigation({ currentView, onNavigate, onOpenFeedback }: NavigationProps) {
4341
const styles = useNavigationStyles()
4442
const { mode, resolved, setMode } = useTheme()
4543

@@ -100,16 +98,6 @@ export default function Navigation({ currentView, onNavigate, onStartTour, onOpe
10098

10199
<div className={styles.spacer} />
102100

103-
{onStartTour && (
104-
<Button
105-
className={styles.navButton}
106-
appearance="subtle"
107-
icon={<QuestionCircleRegular />}
108-
onClick={onStartTour}
109-
title="Take a tour"
110-
aria-label="Take a tour"
111-
/>
112-
)}
113101
<Button
114102
className={styles.navButton}
115103
appearance="subtle"

frontend/src/hooks/useTour.test.ts

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import { ACTIONS, LIFECYCLE, STATUS } from 'react-joyride'
55
import { useTour } from './useTour'
66
import { TOUR_STEPS } from '../components/Tour/tourSteps'
77

8-
const STORAGE_KEY = 'pyrit-tour-completed'
9-
108
// Minimal EventData shape — only fields our handler reads
119
function makeEvent(overrides: Record<string, unknown> = {}) {
1210
return {
@@ -45,17 +43,6 @@ describe('useTour', () => {
4543
jest.restoreAllMocks()
4644
})
4745

48-
it('returns hasCompletedTour=false when localStorage is empty', () => {
49-
const { result } = renderHook(() => useTour(onNavigate, true, 'home'))
50-
expect(result.current.hasCompletedTour).toBe(false)
51-
})
52-
53-
it('returns hasCompletedTour=true when localStorage flag is set', () => {
54-
localStorage.setItem(STORAGE_KEY, 'true')
55-
const { result } = renderHook(() => useTour(onNavigate, true, 'home'))
56-
expect(result.current.hasCompletedTour).toBe(true)
57-
})
58-
5946
it('startTour sets run=true immediately when already on home', () => {
6047
const { result } = renderHook(() => useTour(onNavigate, true, 'home'))
6148

@@ -116,7 +103,7 @@ describe('useTour', () => {
116103
expect(result.current.tourProps.stepIndex).toBe(0)
117104
})
118105

119-
it('stops tour on ACTIONS.CLOSE and persists to localStorage', () => {
106+
it('stops tour on ACTIONS.CLOSE', () => {
120107
const { result } = renderHook(() => useTour(onNavigate, true, 'home'))
121108

122109
act(() => { result.current.startTour() })
@@ -125,9 +112,7 @@ describe('useTour', () => {
125112
act(() => {
126113
result.current.tourProps.onEvent(makeEvent({ action: ACTIONS.CLOSE }))
127114
})
128-
129115
expect(result.current.tourProps.run).toBe(false)
130-
expect(localStorage.getItem(STORAGE_KEY)).toBe('true')
131116
})
132117

133118
it('stops tour on STATUS.SKIPPED', () => {
@@ -138,9 +123,7 @@ describe('useTour', () => {
138123
act(() => {
139124
result.current.tourProps.onEvent(makeEvent({ status: STATUS.SKIPPED }))
140125
})
141-
142126
expect(result.current.tourProps.run).toBe(false)
143-
expect(localStorage.getItem(STORAGE_KEY)).toBe('true')
144127
})
145128

146129
it('stops tour on STATUS.FINISHED', () => {
@@ -257,9 +240,7 @@ describe('useTour', () => {
257240
index: lastIndex,
258241
}))
259242
})
260-
261243
expect(result.current.tourProps.run).toBe(false)
262-
expect(localStorage.getItem(STORAGE_KEY)).toBe('true')
263244
})
264245

265246
it('clears pending step when tour is cancelled during view switch', () => {

frontend/src/hooks/useTour.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ import { TOUR_STEPS } from '../components/Tour/tourSteps'
77
import TourTooltip from '../components/Tour/TourTooltip'
88
import type { ViewName } from '../components/Sidebar/Navigation'
99

10-
const STORAGE_KEY = 'pyrit-tour-completed'
11-
1210
// Static Joyride config — hoisted to module scope so they're created once,
1311
// not on every render. Joyride compares these by reference internally.
1412
const JOYRIDE_STEPS = [...TOUR_STEPS]
@@ -27,7 +25,7 @@ const JOYRIDE_LOCALE = {
2725

2826
/**
2927
* Manages the onboarding tour lifecycle: step progression, cross-view
30-
* navigation, and localStorage persistence.
28+
* navigation, and Joyride configuration.
3129
*
3230
* Returns props to spread onto `<Joyride>` plus control functions.
3331
*/
@@ -73,8 +71,6 @@ export function useTour(onNavigate: (view: ViewName) => void, isDarkMode: boolea
7371
})
7472
}, [currentView])
7573

76-
const hasCompletedTour = localStorage.getItem(STORAGE_KEY) === 'true'
77-
7874
const startTour = useCallback(() => {
7975
setStepIndex(0)
8076
// If we're already on home, start immediately.
@@ -96,7 +92,6 @@ export function useTour(onNavigate: (view: ViewName) => void, isDarkMode: boolea
9692
pendingStepRef.current = null
9793
switchingViewRef.current = false
9894
onNavigate('home')
99-
localStorage.setItem(STORAGE_KEY, 'true')
10095
}, [onNavigate])
10196

10297
const handleJoyrideEvent = useCallback((data: EventData) => {
@@ -178,8 +173,6 @@ export function useTour(onNavigate: (view: ViewName) => void, isDarkMode: boolea
178173
return {
179174
/** Call to start (or restart) the tour from step 1 on the Home view. */
180175
startTour,
181-
/** Whether the user has completed the tour at least once. */
182-
hasCompletedTour,
183176
/** Props to spread onto the `<Joyride>` component. */
184177
tourProps,
185178
}

0 commit comments

Comments
 (0)