Skip to content

feat: Course outline restructure with new plugin slots #1920

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.

Sign up for GitHub

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

Open
xitij2000 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: Course outline restructure with new plugin slots #1920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7053d03
refactor: Separate course outline data and sidebar context logic
xitij2000 Jul 17, 2026
ed7fec6
tests: fix tests
xitij2000 Aug 20, 2026
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
81 changes: 62 additions & 19 deletions src/courseware/course/sidebar/ARCHITECTURE.md
View file
Open in desktop

@brian-smith-tcril brian-smith-tcril Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I haven't read through this file fully but it looks like it might need to be updated, so this is just a note to make sure this file accurately reflects the current state of the repo with the changes in this PR.

Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ The Learning MFE uses a **two-sidebar system** where a left sidebar (Course Outl

### LEFT SIDEBAR (Course Outline)
- **Location**: Left side of screen, adjacent to course content
- **Component**: `CourseOutlineTray` (rendered via `CourseOutlineSidebarSlot`)
- **Component**: `CourseOutlineTray` (rendered via `CourseOutlineSidebarSlot`). This is now a thin wrapper (`CourseOutlineTray.tsx`) that gates on sidebar state and renders the presentational `CourseOutline` (`CourseOutline.tsx`).
- **Purpose**: Navigation - displays course structure, sequences, and units
- **State Management**: Uses `useCourseOutlineSidebar()` hook
- **State Management**: Split across two hooks — `useCourseOutlineData()` (course data + unit-click tracking) and `useCourseOutlineSidebar()` (sidebar open/collapse state)
- **ID**: `WIDGETS.COURSE_OUTLINE`
- **Rendering**: Only renders when `currentSidebar === 'COURSE_OUTLINE'`
- **Trigger**: `CourseOutlineTrigger` (separate location in mobile/desktop toolbar)
Expand Down Expand Up @@ -200,22 +200,36 @@ if (!firstAvailable) {
}
```

### useCourseOutlineSidebar Hook
### Course Outline Hooks (`hooks.js`)

The former single `useCourseOutlineSidebar` hook is split into two, separating
course data from sidebar context so the outline can be reused outside a sidebar:

#### `useCourseOutlineData()`

**Responsibilities:**
- Provide course outline data from Redux: `sections`, `sequences`, `units`, `courseOutlineStatus`, `activeSequenceId`, `sequenceStatus`
- Provide `isEnabledCompletionTracking` and `isActiveEntranceExam`
- Provide `handleUnitClick` (analytics + `checkBlockCompletion`) and trigger the outline-structure load effect
- Reads Redux + `useParams` only — **does not** read `SidebarContext`, so it is reusable outside a sidebar

#### `useCourseOutlineSidebar()`

**Responsibilities:**
- Detect when RIGHT sidebar has no available panels (`!initialSidebar`)
- Auto-open Course Outline as fallback (unless manually collapsed)
- Handle Course Outline specific interactions (unit clicks, toggle, resize)
- Expose sidebar context state: `currentSidebar`, `shouldDisplayFullScreen`, `handleToggleCollapse`
- Handle the resize → collapse behaviour when the viewport drops below the desktop breakpoint

**Key Logic:**
```javascript
const isOpenSidebar = !initialSidebar && !isCollapsedOutlineSidebar;

useEffect(() => {
if (isOpenSidebar && currentSidebar !== ID) {
toggleSidebar('COURSE_OUTLINE');
}
}, [initialSidebar, unitId]);
useLayoutEffect(() => {
const handleResize = () => {
if (currentSidebar === ID && global.innerWidth < breakpoints.large.maxWidth) {
collapseSidebar();
}
};
global.addEventListener('resize', handleResize);
return () => global.removeEventListener('resize', handleResize);
}, [currentSidebar]);
```

### Sidebar.jsx (RIGHT Sidebar Renderer)
Expand All @@ -231,15 +245,44 @@ if (!currentSidebar || !SIDEBARS || !SIDEBARS[currentSidebar]) {
}
```

### CourseOutlineTray.jsx (LEFT Sidebar Renderer)
### CourseOutlineTray.tsx / CourseOutline.tsx (LEFT Sidebar Renderer)

**Responsibilities:**
- Render course outline navigation
- Only show when `currentSidebar === 'COURSE_OUTLINE'`
The renderer is split into a sidebar-aware wrapper and a presentational component:

**`CourseOutlineTray.tsx` (wrapper):**
- Reads `useCourseOutlineSidebar()`
- Only renders when `currentSidebar === 'COURSE_OUTLINE'`, otherwise `null`
- Renders `<CourseOutline>`, passing `shouldDisplayFullScreen` and `onToggleCollapse`
- Still carries `CourseOutlineTray.ID` and is the component registered in the slot

**Key Logic:**
```javascript
if (isActiveEntranceExam || currentSidebar !== ID) {
if (currentSidebar !== ID) {
return null;
}
return <CourseOutline shouldDisplayFullScreen={shouldDisplayFullScreen} onToggleCollapse={handleToggleCollapse} />;
```

**`CourseOutline.tsx` (presentational):**
- Reads course data via `useCourseOutlineData()` + `useParams()`
- Returns `null` only when `isActiveEntranceExam`
- Renders the heading through `CourseOutlineSidebarHeadingSlot` (see Extension Points below)

### Course Outline Extension Points (Plugin Slots)

The refactor extracts the heading and completion icon into standalone components
exposed through plugin slots, so operators can customise them via `env.config.jsx`:

- **`CourseOutlineSidebarHeadingSlot`** — ID `org.openedx.frontend.learning.course_outline_sidebar_heading.v1`. Wraps the extracted `components/CourseOutlineHeading.tsx`. Props: `isDisplaySequenceLevel`, `backButton?`, `onToggleCollapse?`. See `src/plugin-slots/CourseOutlineSidebarHeadingSlot/README.md`.
- **`CourseOutlineSidebarCompletionIconSlot`** — ID `org.openedx.frontend.learning.course_outline_sidebar_completion_icon.v1`. Wraps `CompletionIcon.tsx` (now TypeScript, exporting `CompletionIconProps`). Consumed by **both** `SidebarSection` and `SidebarSequence`; the `variant` prop (`'section' | 'sequence'`) tells a plugin which location it is rendering, and `active` reflects whether that section/sequence is current. See `src/plugin-slots/CourseOutlineSidebarCompletionIconSlot/README.md`.

The mobile "collapse the outline after selecting a unit" behaviour now lives in
`components/UnitLinkWrapper.tsx`, which consumes both hooks (data for the click
handler, sidebar for `shouldDisplayFullScreen`/`handleToggleCollapse`).

### Styling

The outline sidebar styling (`CourseOutlineTray.scss`) uses Paragon theme tokens
rather than hard-coded colours — e.g. borders use `var(--pgn-color-light-700)`.
The active-section highlight is driven by an `.active-section` class on
`.course-sidebar-section` (`background-color: var(--pgn-color-info-100)`) instead
of an inline `bg-info-100` class on the button.
47 changes: 26 additions & 21 deletions src/courseware/course/sidebar/sidebars/course-outline/CourseOutline.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,43 @@
import { useToggle } from '@openedx/paragon';
import { LOADING } from '@src/constants';

import {
useCourseOutlineData,
} from '@src/courseware/course/sidebar/sidebars/course-outline/hooks';
import PageLoading from '@src/generic/PageLoading';
import { CourseOutlineSidebarHeadingSlot } from '@src/plugin-slots/CourseOutlineSidebarHeadingSlot';
import classNames from 'classnames';
import { useState } from 'react';
import SidebarSection from './components/SidebarSection';
import { useParams } from 'react-router-dom';
import SidebarSequence from './components/SidebarSequence';
import { ID } from './constants';
import { useCourseOutlineSidebar } from './hooks';
import SidebarSection from './components/SidebarSection';
import messages from './messages';

export const CourseOutline = () => {
interface CourseOutlineProps {
shouldDisplayFullScreen?: boolean;
onToggleCollapse?: () => void;
}

interface CoursePageParams extends Record<string, string> {
courseId: string;
unitId: string;
}
Comment on lines +22 to +25

@brian-smith-tcril brian-smith-tcril Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude called this out

The non-optional declarations do nothing. courseId: string; unitId: string come back through Partial<> as string | undefined regardless. That's precisely why the author still had to write courseId! and unitId! at lines 100 and 103 — the interface claims the params are always present, the type system disagrees, and the ! papers over the gap. The interface is stating a guarantee it can't deliver, and the route /course/:courseId/:sequenceId (constants.ts:16) means unitId genuinely is absent sometimes.


export const CourseOutline = ({
shouldDisplayFullScreen = false,
onToggleCollapse,
}: CourseOutlineProps) => {
const intl = useIntl();
const [selectedSection, setSelectedSection] = useState<string | null>(null);
const [isDisplaySequenceLevel, setDisplaySequenceLevel, setDisplaySectionLevel] = useToggle(true);

const { unitId, courseId } = useParams<CoursePageParams>();

@brian-smith-tcril brian-smith-tcril Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't see any test changes in this PR. Were there tests assuming we were getting a unitId from useCourseOutlineSidebar before that need to be updated to have one provided by params?

@xitij2000 xitij2000 Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is a good point. I think the fact that not tests fail after moving the source from Sidebar context to useParams means that the tests were probably not testing how unitId is used.

const {
courseId,
unitId,
currentSidebar,
Comment thread
brian-smith-tcril marked this conversation as resolved.
isActiveEntranceExam,
courseOutlineStatus,
activeSequenceId,
sections,
sequences,
shouldDisplayFullScreen,
handleToggleCollapse,
} = useCourseOutlineSidebar();
isActiveEntranceExam,
} = useCourseOutlineData();

const resolvedSectionId = selectedSection
|| Object.keys(sections).find(
Expand All @@ -47,19 +57,16 @@
setDisplaySequenceLevel();
setSelectedSection(id);
};

const sidebarHeading = (
<CourseOutlineSidebarHeadingSlot
onToggleCollapse={handleToggleCollapse}
onToggleCollapse={onToggleCollapse}
isDisplaySequenceLevel={isDisplaySequenceLevel}
backButton={backButtonTitle ? { title: backButtonTitle, onClick: handleBackToSectionLevel } : undefined}
/>
);

if (isActiveEntranceExam || currentSidebar !== ID) {
if (isActiveEntranceExam) {
return null;
}

if (courseOutlineStatus === LOADING) {
return (
<div className={classNames('outline-sidebar-wrapper', {
Expand Down Expand Up @@ -90,10 +97,10 @@
? sequenceIds.map((sequenceId: string) => (
<SidebarSequence
key={sequenceId}
courseId={courseId!}
courseId={courseId}

Check failure on line 100 in src/courseware/course/sidebar/sidebars/course-outline/CourseOutline.tsx

View workflow job for this annotation

GitHub Actions / tests 

Type 'string | undefined' is not assignable to type 'string'.
sequence={sequences[sequenceId]}
defaultOpen={sequenceId === activeSequenceId}
activeUnitId={unitId!}
activeUnitId={unitId}

Check failure on line 103 in src/courseware/course/sidebar/sidebars/course-outline/CourseOutline.tsx

View workflow job for this annotation

GitHub Actions / tests 

Type 'string | undefined' is not assignable to type 'string'.
/>
))
: sectionsIds.map((sectionId) => (
Expand All @@ -108,5 +115,3 @@
</div>
);
};

export default CourseOutline;
25 changes: 19 additions & 6 deletions src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTray.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { MemoryRouter } from 'react-router-dom';
import { Routes, Route } from 'react-router';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { AppProvider } from '@edx/frontend-platform/react';
Expand All @@ -18,6 +19,7 @@ describe('<CourseOutlineTray />', () => {
let unit;
let unitId;
let courseId;
let sequenceId;
let mockData;

const initTestStore = async (options) => {
Expand All @@ -27,29 +29,30 @@ describe('<CourseOutlineTray />', () => {
[unitId] = Object.keys(state.models.units);

if (Object.keys(state.courseware.courseOutline).length) {
const [activeSequenceId] = Object.keys(state.courseware.courseOutline.sequences);
sequence = state.courseware.courseOutline.sequences[activeSequenceId];
[sequenceId] = Object.keys(state.courseware.courseOutline.sequences);
sequence = state.courseware.courseOutline.sequences[sequenceId];
const activeSectionId = Object.keys(state.courseware.courseOutline.sections)[0];
section = state.courseware.courseOutline.sections[activeSectionId];
[unitId] = sequence.unitIds;
unit = state.courseware.courseOutline.units[unitId];
}

mockData = {
courseId,
unitId,
currentSidebar: outlineSidebarId,
toggleSidebar: jest.fn(),
};
};

function renderWithProvider(testData = {}) {
const path = `/course/${courseId}/${sequenceId}/${unitId}`;
const { container } = render(
<AppProvider store={store} wrapWithRouter={false}>
<IntlProvider locale="en">
<SidebarContext.Provider value={{ ...mockData, ...testData }}>
<MemoryRouter>
<CourseOutlineTray />
<MemoryRouter initialEntries={[path]}>
<Routes>
<Route path="/course/:courseId/:sequenceId/:unitId" element={<CourseOutlineTray />} />
</Routes>
</MemoryRouter>
</SidebarContext.Provider>
</IntlProvider>
Expand Down Expand Up @@ -124,4 +127,14 @@ describe('<CourseOutlineTray />', () => {
await user.click(screen.getByRole('button', { name: new RegExp(`${section.title} , ${courseOutlineMessages.incompleteSection.defaultMessage}`) }));
expect(screen.queryByRole('button', { name: section.title })).toBeInTheDocument();
});

it('highlights the unit matching teh current unitId', async () => {
await initTestStore();
renderWithProvider();
// Check that there is only one link with the highlight class and it's
// the one with the active unit title.
const highlightedItems = screen.getAllByRole('listitem').filter(li => li.classList.contains('bg-info-100'));
expect(highlightedItems).toHaveLength(1);
expect(highlightedItems[0]).toHaveTextContent(unit.title);
});
});
4 changes: 3 additions & 1 deletion src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTray.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import { useCourseOutlineSidebar } from './hooks';
const CourseOutlineTray = () => {
const {
currentSidebar,
shouldDisplayFullScreen,
handleToggleCollapse,
} = useCourseOutlineSidebar();

if (currentSidebar !== ID) {
return null;
}
return <CourseOutline />;
return <CourseOutline shouldDisplayFullScreen={shouldDisplayFullScreen} onToggleCollapse={handleToggleCollapse} />;

@brian-smith-tcril brian-smith-tcril Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude question about this:

Why props here instead of having CourseOutline call useCourseOutlineSidebar() directly? Every other consumer in this subtree — the trigger, UnitLinkWrapper — reads the sidebar hook itself, so CourseOutline becomes the only component being fed these two values from outside. If the intent is to make CourseOutline renderable outside a sidebar, could the props fall back to the context values when they aren't passed, rather than to false/undefined? That way the sidebar case keeps working through the hook as it does today (and CourseOutlineTray doesn't need to pass anything at all), and the props become an explicit override for the out-of-sidebar case.

@xitij2000 xitij2000 Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Having CourseOuline call useCourseOutlineSidebar is the exact reason for this refactoring in the first place, we want to make it independent of any sidebar logic. Having it still get it from the SidebarContext would partially defeat the purpose of the split.

@brian-smith-tcril brian-smith-tcril Aug 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Having CourseOuline call useCourseOutlineSidebar is the exact reason for this refactoring in the first place, we want to make it independent of any sidebar logic. Having it still get it from the SidebarContext would partially defeat the purpose of the split.

Could you elaborate on this a bit? My feeling is that regardless of where CourseOutline gets shouldDisplayFullScreen and handleToggleCollapse from, by caring about those at all it's including "sidebar logic."

I'm open to these being passed as props, I just want to understand the motivation a bit better.

@xitij2000 xitij2000 Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see your point, that is how those values are currently used. However for a client we extracted this component and injected it in the header as a dropdown. It can still use the toggle collapse, but its context is different from that of a sidebar. This work was triggered by the need for hosting this component outside the sidebar. Having the values passed explicitly seemed to be a cleaner separation.

That said it's still in the sidebars folder so without further refactoring it's still somewhat tied to the sidebar and keeping the values by default won't break anything.

We could do further refactoring to move this out and make the fullscreen code part of the sidebar wrapper rather than the outline component, but I think for now I can make these params default to the sidebar context values.

};

CourseOutlineTray.ID = ID;
Expand Down
4 changes: 2 additions & 2 deletions src/courseware/course/sidebar/sidebars/course-outline/CourseOutlineTrigger.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ import { useIntl } from '@edx/frontend-platform/i18n';
import { IconButton } from '@openedx/paragon';
import { MenuOpen as MenuOpenIcon } from '@openedx/paragon/icons';

import { useCourseOutlineSidebar } from './hooks';
import { useCourseOutlineData, useCourseOutlineSidebar } from './hooks';
import { ID } from './constants';
import messages from './messages';

const CourseOutlineTrigger = ({ isMobileView }) => {
const intl = useIntl();
const { isActiveEntranceExam } = useCourseOutlineData();
const {
currentSidebar,
shouldDisplayFullScreen,
handleToggleCollapse,
isActiveEntranceExam,
} = useCourseOutlineSidebar();

const isDisplayForDesktopView = !isMobileView && !shouldDisplayFullScreen && currentSidebar !== ID;
Expand Down
3 changes: 2 additions & 1 deletion src/courseware/course/sidebar/sidebars/course-outline/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,5 @@ Unlike right-sidebar widgets, the course outline is **not** registered in the `S
| `CourseOutlineTray` | Main tray panel component |
| `CourseOutlineTrigger` | Collapse/expand trigger button |
| `ID` | Widget sentinel ID: `'COURSE_OUTLINE'` |
| `useCourseOutlineSidebar` | Hook providing all tray state and handlers |
| `useCourseOutlineData` | Hook providing course outline data (sections, sequences, units, status) and unit-click tracking; usable outside a sidebar context |
| `useCourseOutlineSidebar` | Hook providing sidebar open/collapse state and handlers (reads `SidebarContext`) |
4 changes: 2 additions & 2 deletions src/courseware/course/sidebar/sidebars/course-outline/components/SidebarSection.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { Button, Icon } from '@openedx/paragon';
import { ChevronRight as ChevronRightIcon } from '@openedx/paragon/icons';

import courseOutlineMessages from '@src/course-home/outline-tab/messages';
import { useCourseOutlineSidebar } from '../hooks';
import { useCourseOutlineData } from '../hooks';

const SidebarSection = ({ section, handleSelectSection }) => {
const intl = useIntl();
Expand All @@ -20,7 +20,7 @@ const SidebarSection = ({ section, handleSelectSection }) => {
completionStat,
} = section;

const { activeSequenceId, isEnabledCompletionTracking } = useCourseOutlineSidebar();
const { activeSequenceId, isEnabledCompletionTracking } = useCourseOutlineData();
const isActiveSection = sequenceIds.includes(activeSequenceId);

const sectionTitle = (
Expand Down
4 changes: 2 additions & 2 deletions src/courseware/course/sidebar/sidebars/course-outline/components/SidebarSequence.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { useIntl } from '@edx/frontend-platform/i18n';
import { Collapsible } from '@openedx/paragon';

import courseOutlineMessages from '@src/course-home/outline-tab/messages';
import { useCourseOutlineSidebar } from '../hooks';
import { useCourseOutlineData } from '../hooks';
import SidebarUnit from './SidebarUnit';
import { UNIT_ICON_TYPES } from './UnitIcon';

Expand All @@ -30,7 +30,7 @@ const SidebarSequence = ({
} = sequence;

const [open, setOpen] = useState(defaultOpen);
const { activeSequenceId, units, isEnabledCompletionTracking } = useCourseOutlineSidebar();
const { activeSequenceId, units, isEnabledCompletionTracking } = useCourseOutlineData();
const isActiveSequence = id === activeSequenceId;

const sectionTitle = (
Expand Down
14 changes: 11 additions & 3 deletions src/courseware/course/sidebar/sidebars/course-outline/components/UnitLinkWrapper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { Link, useLocation } from 'react-router-dom';

import { useCourseOutlineSidebar } from '../hooks';
import { useCourseOutlineData, useCourseOutlineSidebar } from '../hooks';

interface Props {
courseId: string;
Expand All @@ -26,17 +26,25 @@ const UnitLinkWrapper: React.FC<Props> = ({
courseId,
children,
}) => {
const { handleUnitClick } = useCourseOutlineSidebar();
const { handleUnitClick } = useCourseOutlineData();
const { shouldDisplayFullScreen, handleToggleCollapse } = useCourseOutlineSidebar();
const { pathname } = useLocation();
const isPreview = pathname.startsWith('/preview');
const baseUrl = `/course/${courseId}/${sequenceId}/${id}`;
const link = isPreview ? `/preview${baseUrl}` : baseUrl;
const handleClick = React.useCallback(() => {
// Hide the sidebar after selecting a unit on a mobile device.
if (shouldDisplayFullScreen) {
handleToggleCollapse();
}
handleUnitClick({ sequenceId, activeUnitId, id });
}, [handleUnitClick, sequenceId, activeUnitId, id, shouldDisplayFullScreen, handleToggleCollapse]);
Comment thread
brian-smith-tcril marked this conversation as resolved.
Comment on lines +35 to +41

@brian-smith-tcril brian-smith-tcril Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking at this block a few things stand out to me:

  • It's no longer inline
    • This makes sense, we used to have the handleToggleCollapse in handleUnitClick, now that needs to be called here instead, making it a named function seems reasonable.
  • The if (shouldDisplayFullScreen) { handleToggleCollapse block is before handleUnitClick
    • That block was at the end of handleUnitClick before. I don't think there's a behavioral difference with it moving, but it stood out to me as a difference to investigate.
  • It is now using useCallback instead of a plain arrow function.
    • It's not clear to me what this is buying us. I'm open to the change if there's clear justification, but I'd think sticking to a plain arrow function here would be fine.

Claude dive into why useCallback isn't buying us anything

  1. Something re-renders UnitLinkWrapper.
  2. UnitLinkWrapper() runs again, so useCourseOutlineData() runs again.
  3. That hook's line const handleUnitClick = (...) => {...} executes again → allocates a different function object than last render.
  4. useCallback compares this render's deps to last render's with Object.is. handleUnitClick is a different object, so that check fails.
  5. Cache miss → useCallback returns a newly created handleClick.


return (
<Link
to={link}
className="row w-100 m-0 d-flex align-items-center text-gray-700"
onClick={() => handleUnitClick({ sequenceId, activeUnitId, id })}
onClick={handleClick}
>
{children}
</Link>
Expand Down
Loading
Loading