Skip to content

Commit 063528e

Browse files
committed
refactor(outline-sidebar): centralize back navigation
1 parent d66a42d commit 063528e

7 files changed

Lines changed: 72 additions & 130 deletions

File tree

src/course-outline/outline-sidebar/AddSidebar.tsx

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import { COURSE_BLOCK_NAMES } from '@src/constants';
2929
import { BlockCardButton } from '@src/generic/sidebar/BlockCardButton';
3030
import AlertMessage from '@src/generic/alert-message';
3131
import { useCourseItemData } from '@src/course-outline/data/apiHooks';
32-
import { navigateBackFromSelection } from './back-navigation';
32+
import { useBackNavigation } from './back-navigation';
3333
import { useOutlineSidebarContext } from './OutlineSidebarContext';
3434
import messages from './messages';
3535

@@ -360,12 +360,10 @@ const AddTabs = () => {
360360
export const AddSidebar = () => {
361361
const intl = useIntl();
362362
const { courseDetails } = useCourseAuthoringContext();
363-
const { sections } = useCourseOutlineContext();
364363
const {
365364
isCurrentFlowOn,
366365
currentFlow,
367366
currentItemData,
368-
clearSelection,
369367
stopCurrentFlow,
370368
selectedContainerState,
371369
openContainerSidebar,
@@ -389,14 +387,13 @@ export const AddSidebar = () => {
389387
courseDetails,
390388
]);
391389

390+
const handleSelectionBack = useBackNavigation({
391+
openContainer: openContainerSidebar,
392+
});
393+
392394
const handleBack = () => {
393395
stopCurrentFlow();
394-
navigateBackFromSelection({
395-
selectedContainerState,
396-
sections,
397-
openContainer: openContainerSidebar,
398-
clearSelection,
399-
});
396+
handleSelectionBack();
400397
};
401398

402399
return (

src/course-outline/outline-sidebar/OutlineAlignSidebar.test.tsx

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ describe('OutlineAlignSidebar', () => {
1919
const setCurrentSelection = jest.fn();
2020
const clearSelection = jest.fn();
2121
const openContainerSidebar = jest.fn();
22+
const sectionId = 'block-v1:test+course+run+type@chapter+block@section-1';
23+
const subsectionId = 'block-v1:test+course+run+type@sequential+block@subsection-1';
24+
const unitId = 'block-v1:test+course+run+type@vertical+block@unit-1';
2225

2326
beforeEach(() => {
2427
initializeMocks();
@@ -36,10 +39,10 @@ describe('OutlineAlignSidebar', () => {
3639
setCurrentSelection,
3740
sections: [
3841
{
39-
id: 'section-1',
42+
id: sectionId,
4043
childInfo: {
4144
children: [
42-
{ id: 'subsection-1', childInfo: { children: [{ id: 'unit-1' }] } },
45+
{ id: subsectionId, childInfo: { children: [{ id: unitId }] } },
4346
],
4447
},
4548
},
@@ -109,9 +112,9 @@ describe('OutlineAlignSidebar', () => {
109112
.spyOn(OutlineSidebarContext, 'useOutlineSidebarContext')
110113
.mockReturnValue({
111114
selectedContainerState: {
112-
currentId: 'unit-1',
113-
subsectionId: 'subsection-1',
114-
sectionId: 'section-1',
115+
currentId: unitId,
116+
subsectionId,
117+
sectionId,
115118
},
116119
clearSelection,
117120
openContainerSidebar,
@@ -122,11 +125,11 @@ describe('OutlineAlignSidebar', () => {
122125
const backButton = await screen.findByRole('button', { name: /back/i });
123126
backButton.click();
124127

125-
expect(openContainerSidebar).toHaveBeenCalledWith('subsection-1', 'subsection-1', 'section-1', 0);
128+
expect(openContainerSidebar).toHaveBeenCalledWith(subsectionId, subsectionId, sectionId, 0);
126129
expect(setCurrentSelection).toHaveBeenCalledWith({
127-
currentId: 'subsection-1',
128-
subsectionId: 'subsection-1',
129-
sectionId: 'section-1',
130+
currentId: subsectionId,
131+
subsectionId,
132+
sectionId,
130133
index: 0,
131134
});
132135
});
@@ -136,8 +139,8 @@ describe('OutlineAlignSidebar', () => {
136139
.spyOn(OutlineSidebarContext, 'useOutlineSidebarContext')
137140
.mockReturnValue({
138141
selectedContainerState: {
139-
currentId: 'section-1',
140-
sectionId: 'section-1',
142+
currentId: sectionId,
143+
sectionId,
141144
},
142145
clearSelection,
143146
openContainerSidebar,

src/course-outline/outline-sidebar/OutlineAlignSidebar.tsx

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,25 @@ import { useContentData } from '@src/content-tags-drawer/data/apiHooks';
22
import { useCourseAuthoringContext } from '@src/CourseAuthoringContext';
33
import { useCourseOutlineContext } from '@src/course-outline/CourseOutlineContext';
44
import { AlignSidebar } from '@src/generic/sidebar/AlignSidebar';
5-
import { navigateBackFromSelection } from './back-navigation';
5+
import { useBackNavigation } from './back-navigation';
66
import { useOutlineSidebarContext } from './OutlineSidebarContext';
77

88
/**
99
* Align sidebar for course or selected containers.
1010
*/
1111
export const OutlineAlignSidebar = () => {
1212
const { courseId } = useCourseAuthoringContext();
13-
const { setCurrentSelection, sections } = useCourseOutlineContext();
14-
const { selectedContainerState, clearSelection, openContainerSidebar } = useOutlineSidebarContext();
13+
const { setCurrentSelection } = useCourseOutlineContext();
14+
const { selectedContainerState, openContainerSidebar } = useOutlineSidebarContext();
1515

1616
const sidebarContentId = selectedContainerState?.currentId || courseId;
1717

1818
const { data: contentData } = useContentData(sidebarContentId);
1919

20-
const handleBack = () => {
21-
navigateBackFromSelection({
22-
selectedContainerState,
23-
sections,
24-
openContainer: openContainerSidebar,
25-
clearSelection,
26-
onSelectionChange: setCurrentSelection,
27-
});
28-
};
20+
const handleBack = useBackNavigation({
21+
openContainer: openContainerSidebar,
22+
onSelectionChange: setCurrentSelection,
23+
});
2924

3025
return (
3126
<AlignSidebar

src/course-outline/outline-sidebar/back-navigation.test.ts

Lines changed: 5 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { SelectionState } from '@src/data/types';
2-
import { getBackSelectionState, navigateBackFromSelection } from './back-navigation';
2+
import { getBackSelectionState, openSelectionState } from './back-navigation';
33

44
describe('back-navigation', () => {
55
const sections = [
@@ -102,53 +102,18 @@ describe('back-navigation', () => {
102102
});
103103
});
104104

105-
describe('navigateBackFromSelection', () => {
106-
it('opens parent and propagates selection when back target exists', () => {
105+
describe('openSelectionState', () => {
106+
it('opens container with SelectionState payload', () => {
107107
const openContainer = jest.fn();
108-
const clearSelection = jest.fn();
109-
const onSelectionChange = jest.fn();
110-
111-
navigateBackFromSelection({
112-
selectedContainerState: {
113-
currentId: 'unit-1',
114-
subsectionId: 'subsection-1',
115-
sectionId: 'section-1',
116-
},
117-
sections,
118-
openContainer,
119-
clearSelection,
120-
onSelectionChange,
121-
});
122108

123-
expect(openContainer).toHaveBeenCalledWith('subsection-1', 'subsection-1', 'section-1', 0);
124-
expect(onSelectionChange).toHaveBeenCalledWith({
109+
openSelectionState(openContainer, {
125110
currentId: 'subsection-1',
126111
subsectionId: 'subsection-1',
127112
sectionId: 'section-1',
128113
index: 0,
129114
});
130-
expect(clearSelection).not.toHaveBeenCalled();
131-
});
132115

133-
it('clears selection and propagates undefined when no back target', () => {
134-
const openContainer = jest.fn();
135-
const clearSelection = jest.fn();
136-
const onSelectionChange = jest.fn();
137-
138-
navigateBackFromSelection({
139-
selectedContainerState: {
140-
currentId: 'section-1',
141-
sectionId: 'section-1',
142-
},
143-
sections,
144-
openContainer,
145-
clearSelection,
146-
onSelectionChange,
147-
});
148-
149-
expect(clearSelection).toHaveBeenCalled();
150-
expect(onSelectionChange).toHaveBeenCalledWith(undefined);
151-
expect(openContainer).not.toHaveBeenCalled();
116+
expect(openContainer).toHaveBeenCalledWith('subsection-1', 'subsection-1', 'section-1', 0);
152117
});
153118
});
154119
});

src/course-outline/outline-sidebar/back-navigation.ts

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1+
import { useCallback } from 'react';
2+
import { useCourseOutlineContext } from '@src/course-outline/CourseOutlineContext';
13
import { SelectionState, XBlock } from '@src/data/types';
4+
import { useCourseItemData } from '@src/course-outline/data/apiHooks';
5+
import { useOutlineSidebarContext } from './OutlineSidebarContext';
26

37
/**
48
* Function signature shared by sidebar container openers.
@@ -15,22 +19,11 @@ type OpenContainerFn = (
1519
) => void;
1620

1721
/**
18-
* Inputs for generic "back" navigation resolution.
22+
* Inputs for useBackNavigation hook.
1923
*/
20-
type NavigateBackOptions = {
21-
/** Current selection state in sidebar context. */
22-
selectedContainerState: SelectionState | undefined;
23-
/** Sections list from course outline context (used for parent index lookup). */
24-
sections: Array<XBlock>;
24+
type UseBackNavigationOptions = {
2525
/** Callback used to open a container sidebar (align/info/add flavors). */
2626
openContainer: OpenContainerFn;
27-
/** Fallback action when no parent target exists. */
28-
clearSelection: () => void;
29-
/**
30-
* Optional authoritative selected section payload.
31-
* Useful when section list is partial/minimal in tests or callers.
32-
*/
33-
selectedSection?: XBlock;
3427
/** Optional hook to sync external selection state (e.g. align view selection). */
3528
onSelectionChange?: (selectionState?: SelectionState) => void;
3629
};
@@ -51,31 +44,34 @@ export const openSelectionState = (
5144
};
5245

5346
/**
54-
* Execute generic back navigation for outline sidebars.
47+
* Hook that returns standardized sidebar "back" handler.
5548
*
56-
* Flow:
57-
* 1) Compute parent selection state.
58-
* 2) If parent exists, open parent and optionally sync external selection.
59-
* 3) Else clear selection and optionally sync undefined.
49+
* It pulls required state from context/hooks:
50+
* - selectedContainerState + clearSelection (sidebar context)
51+
* - sections (course outline context)
52+
* - selected section data via useCourseItemData (React Query cache)
6053
*/
61-
export const navigateBackFromSelection = ({
62-
selectedContainerState,
63-
sections,
64-
openContainer,
65-
clearSelection,
66-
selectedSection,
67-
onSelectionChange,
68-
}: NavigateBackOptions) => {
69-
const backSelectionState = getBackSelectionState(selectedContainerState, sections, selectedSection);
54+
export const useBackNavigation = ({ openContainer, onSelectionChange }: UseBackNavigationOptions) => {
55+
const { sections } = useCourseOutlineContext();
56+
const { selectedContainerState, clearSelection } = useOutlineSidebarContext();
57+
const { data: selectedSection } = useCourseItemData<XBlock>(
58+
selectedContainerState?.sectionId,
59+
undefined,
60+
Boolean(selectedContainerState?.sectionId),
61+
);
7062

71-
if (backSelectionState) {
72-
openSelectionState(openContainer, backSelectionState);
73-
onSelectionChange?.(backSelectionState);
74-
return;
75-
}
63+
return useCallback(() => {
64+
const backSelectionState = getBackSelectionState(selectedContainerState, sections, selectedSection);
65+
66+
if (backSelectionState) {
67+
openSelectionState(openContainer, backSelectionState);
68+
onSelectionChange?.(backSelectionState);
69+
return;
70+
}
7671

77-
clearSelection();
78-
onSelectionChange?.(undefined);
72+
clearSelection();
73+
onSelectionChange?.(undefined);
74+
}, [selectedContainerState, sections, selectedSection, openContainer, onSelectionChange, clearSelection]);
7975
};
8076

8177
/**

src/course-outline/outline-sidebar/info-sidebar/SubsectionInfoSidebar.tsx

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { useOutlineSidebarContext } from '@src/course-outline/outline-sidebar/Ou
1515
import { getLibraryId } from '@src/generic/key-utils';
1616
import { possibleSubsectionMoves } from '@src/course-outline/drag-helper/utils';
1717
import { XBlock } from '@src/data/types';
18-
import { navigateBackFromSelection } from '../back-navigation';
18+
import { useBackNavigation } from '../back-navigation';
1919

2020
import { InfoSection } from './InfoSection';
2121
import { PublishButon } from './PublishButon';
@@ -27,7 +27,6 @@ export const SubsectionSidebar = () => {
2727
const navigate = useNavigate();
2828

2929
const {
30-
clearSelection,
3130
currentTabKey,
3231
setCurrentTabKey,
3332
selectedContainerState,
@@ -60,6 +59,10 @@ export const SubsectionSidebar = () => {
6059
} = useCourseOutlineContext();
6160
const sectionIndex = sections.findIndex((s) => s.id === selectedContainerState?.sectionId);
6261

62+
const handleBack = useBackNavigation({
63+
openContainer: openContainerInfoSidebar,
64+
});
65+
6366
const handlePublish = () => {
6467
if (selectedContainerState?.sectionId && subsectionData?.hasChanges) {
6568
openPublishModal({
@@ -127,16 +130,6 @@ export const SubsectionSidebar = () => {
127130
}
128131
};
129132

130-
const handleBack = () => {
131-
navigateBackFromSelection({
132-
selectedContainerState,
133-
sections,
134-
selectedSection: section,
135-
openContainer: openContainerInfoSidebar,
136-
clearSelection,
137-
});
138-
};
139-
140133
return (
141134
<>
142135
<SidebarTitle

src/course-outline/outline-sidebar/info-sidebar/UnitInfoSidebar.tsx

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import { InfoSection } from './InfoSection';
3535
import { useClipboard } from '@src/generic/clipboard';
3636
import { ToastContext } from '@src/generic/toast-context';
3737
import { XBlock } from '@src/data/types';
38-
import { navigateBackFromSelection } from '../back-navigation';
38+
import { useBackNavigation } from '../back-navigation';
3939
interface Props {
4040
unitId: string;
4141
}
@@ -72,7 +72,6 @@ export const UnitSidebar = () => {
7272
const navigate = useNavigate();
7373
const {
7474
selectedContainerState,
75-
clearSelection,
7675
currentTabKey,
7776
setCurrentTabKey,
7877
setSelectedContainerState,
@@ -113,6 +112,10 @@ export const UnitSidebar = () => {
113112
const { copyToClipboard } = useClipboard();
114113
const { showToast } = useContext(ToastContext);
115114

115+
const handleBack = useBackNavigation({
116+
openContainer: openContainerInfoSidebar,
117+
});
118+
116119
const handlePublish = () => {
117120
if (unitData?.hasChanges) {
118121
openPublishModal({
@@ -211,16 +214,6 @@ export const UnitSidebar = () => {
211214
showToast(intl.formatMessage(messages.locationCopiedText));
212215
};
213216

214-
const handleBack = () => {
215-
navigateBackFromSelection({
216-
selectedContainerState,
217-
sections,
218-
selectedSection: section,
219-
openContainer: openContainerInfoSidebar,
220-
clearSelection,
221-
});
222-
};
223-
224217
return (
225218
<>
226219
<SidebarTitle

0 commit comments

Comments
 (0)