From e4eeb373c271a19fd42d71c18686589c49621b84 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Tue, 11 Aug 2026 15:32:03 -0400 Subject: [PATCH] refactor: simplify the course-home tour button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the vestigial `metadataModel` prop from the shared TabPage. Its only live effect was gating the screen-reader-only launch-tour button, which is only ever real on the outline tab — gate that on `activeTabSlug === 'outline'` instead and rename the helper to `renderSrOnlyTourButton`. Drop `metadataModel` from LoadedTabPage and its dead pass-through to StreakCelebrationModal, and from the DatesTab / CoursewareContainer / TabContainer call sites. Move LaunchCourseHomeTourButton's `courseId` from the Redux slice to `useParams`. Co-Authored-By: Claude Opus 4.8 --- src/course-home/dates-tab/DatesTab.jsx | 1 - src/course-home/outline-tab/OutlineTab.test.jsx | 4 ++-- src/course-home/progress-tab/ProgressTab.test.jsx | 8 ++++---- src/courseware/CoursewareContainer.jsx | 1 - src/product-tours/ProductTours.test.jsx | 2 +- .../LaunchCourseHomeTourButton.jsx | 6 ++---- .../streak-celebration/StreakCelebrationModal.jsx | 3 +-- .../StreakCelebrationModal.test.jsx | 1 - src/tab-page/LoadedTabPage.test.jsx | 2 +- src/tab-page/LoadedTabPage.tsx | 3 --- src/tab-page/TabContainer.jsx | 1 - src/tab-page/TabPage.test.jsx | 14 ++++++++++++++ src/tab-page/TabPage.tsx | 13 +++++++------ 13 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/course-home/dates-tab/DatesTab.jsx b/src/course-home/dates-tab/DatesTab.jsx index e9248a3f59..85f50688cb 100644 --- a/src/course-home/dates-tab/DatesTab.jsx +++ b/src/course-home/dates-tab/DatesTab.jsx @@ -49,7 +49,6 @@ const DatesTab = () => { activeTabSlug="dates" courseId={courseId} courseStatus={{ metadataQuery, tabDataQuery }} - metadataModel="courseHomeMeta" >
{intl.formatMessage(messages.title)} diff --git a/src/course-home/outline-tab/OutlineTab.test.jsx b/src/course-home/outline-tab/OutlineTab.test.jsx index 5e9bffaf33..8613916368 100644 --- a/src/course-home/outline-tab/OutlineTab.test.jsx +++ b/src/course-home/outline-tab/OutlineTab.test.jsx @@ -666,7 +666,7 @@ describe('Outline Tab', () => { }, }); await executeThunk(thunks.fetchOutlineTab(courseId), store.dispatch); - await act(async () => render(..., { store })); + await act(async () => render(..., { store })); const instructorToolbar = await screen.getByTestId('instructor-toolbar'); expect(instructorToolbar).toBeInTheDocument(); expect(screen.getByText('This learner no longer has access to this course. Their access expired on', { exact: false })).toBeInTheDocument(); @@ -682,7 +682,7 @@ describe('Outline Tab', () => { }, }); await executeThunk(thunks.fetchOutlineTab(courseId), store.dispatch); - await act(async () => render(..., { store })); + await act(async () => render(..., { store })); const instructorToolbar = await screen.getByTestId('instructor-toolbar'); expect(instructorToolbar).toBeInTheDocument(); expect(screen.queryByText('This learner no longer has access to this course. Their access expired on', { exact: false })).not.toBeInTheDocument(); diff --git a/src/course-home/progress-tab/ProgressTab.test.jsx b/src/course-home/progress-tab/ProgressTab.test.jsx index 09d4431ad5..7c1b2ce343 100644 --- a/src/course-home/progress-tab/ProgressTab.test.jsx +++ b/src/course-home/progress-tab/ProgressTab.test.jsx @@ -1432,7 +1432,7 @@ describe('Progress Tab', () => { }, }); await executeThunk(thunks.fetchProgressTab(courseId), store.dispatch); - await act(async () => render(..., { store })); + await act(async () => render(..., { store })); expect(screen.getByTestId('instructor-toolbar')).toBeInTheDocument(); expect(screen.getByText('This learner no longer has access to this course. Their access expired on', { exact: false })).toBeInTheDocument(); expect(screen.getByText('1/1/2020', { exact: false })).toBeInTheDocument(); @@ -1446,7 +1446,7 @@ describe('Progress Tab', () => { }, }); await executeThunk(thunks.fetchProgressTab(courseId), store.dispatch); - await act(async () => render(..., { store })); + await act(async () => render(..., { store })); expect(screen.queryByText('This learner no longer has access to this course. Their access expired on', { exact: false })).not.toBeInTheDocument(); expect(screen.queryByText('1/1/2020', { exact: false })).not.toBeInTheDocument(); }); @@ -1461,7 +1461,7 @@ describe('Progress Tab', () => { start: '2999-01-01T00:00:00Z', }); await executeThunk(thunks.fetchProgressTab(courseId), store.dispatch); - await act(async () => render(..., { store })); + await act(async () => render(..., { store })); expect(screen.getByTestId('instructor-toolbar')).toBeInTheDocument(); expect(screen.getByText('This learner does not yet have access to this course. The course starts on', { exact: false })).toBeInTheDocument(); expect(screen.getByText('1/1/2999', { exact: false })).toBeInTheDocument(); @@ -1474,7 +1474,7 @@ describe('Progress Tab', () => { start: '2999-01-01T00:00:00Z', }); await executeThunk(thunks.fetchProgressTab(courseId), store.dispatch); - await act(async () => render(..., { store })); + await act(async () => render(..., { store })); expect(screen.queryByText('This learner does not yet have access to this course. The course starts on', { exact: false })).not.toBeInTheDocument(); expect(screen.queryByText('1/1/2999', { exact: false })).not.toBeInTheDocument(); }); diff --git a/src/courseware/CoursewareContainer.jsx b/src/courseware/CoursewareContainer.jsx index 41aa01f227..a715a7b955 100644 --- a/src/courseware/CoursewareContainer.jsx +++ b/src/courseware/CoursewareContainer.jsx @@ -345,7 +345,6 @@ class CoursewareContainer extends Component { courseId={courseId} unitId={routeUnitId} courseStatus={courseStatus} - metadataModel="coursewareMeta" > { await executeThunk(courseHomeThunks.fetchOutlineTab(courseId), store.dispatch); render( - + , diff --git a/src/product-tours/newUserCourseHomeTour/LaunchCourseHomeTourButton.jsx b/src/product-tours/newUserCourseHomeTour/LaunchCourseHomeTourButton.jsx index f2697fd822..5e3805e4d8 100644 --- a/src/product-tours/newUserCourseHomeTour/LaunchCourseHomeTourButton.jsx +++ b/src/product-tours/newUserCourseHomeTour/LaunchCourseHomeTourButton.jsx @@ -1,6 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { useSelector } from 'react-redux'; +import { useParams } from 'react-router-dom'; import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; @@ -15,9 +15,7 @@ import messages from '../messages'; const LaunchCourseHomeTourButton = ({ srOnly }) => { const intl = useIntl(); - const { - courseId, - } = useSelector(state => state.courseHome); + const { courseId } = useParams(); const { org, diff --git a/src/shared/streak-celebration/StreakCelebrationModal.jsx b/src/shared/streak-celebration/StreakCelebrationModal.jsx index b5764cf7f4..dc69b29fd4 100644 --- a/src/shared/streak-celebration/StreakCelebrationModal.jsx +++ b/src/shared/streak-celebration/StreakCelebrationModal.jsx @@ -54,7 +54,7 @@ const CloseText = ({ intl }) => ( ); const StreakModal = ({ - courseId, metadataModel, streakLengthToCelebrate, isStreakCelebrationOpen, + courseId, streakLengthToCelebrate, isStreakCelebrationOpen, closeStreakCelebration, streakDiscountCouponEnabled, verifiedMode, ...rest }) => { const intl = useIntl(); @@ -247,7 +247,6 @@ StreakModal.defaultProps = { StreakModal.propTypes = { courseId: PropTypes.string.isRequired, - metadataModel: PropTypes.string.isRequired, streakLengthToCelebrate: PropTypes.number, isStreakCelebrationOpen: PropTypes.bool, closeStreakCelebration: PropTypes.func.isRequired, diff --git a/src/shared/streak-celebration/StreakCelebrationModal.test.jsx b/src/shared/streak-celebration/StreakCelebrationModal.test.jsx index c71b9946a4..d8d24ee7b8 100644 --- a/src/shared/streak-celebration/StreakCelebrationModal.test.jsx +++ b/src/shared/streak-celebration/StreakCelebrationModal.test.jsx @@ -61,7 +61,6 @@ describe('Loaded Tab Page', () => { closeStreakCelebration: jest.fn(), courseId: courseMetadata.id, isStreakCelebrationOpen: true, - metadataModel: 'coursewareMeta', streakLengthToCelebrate: 3, verifiedMode: camelCaseObject(courseHomeMetadata.verified_mode), }; diff --git a/src/tab-page/LoadedTabPage.test.jsx b/src/tab-page/LoadedTabPage.test.jsx index ec521cca32..084b86c917 100644 --- a/src/tab-page/LoadedTabPage.test.jsx +++ b/src/tab-page/LoadedTabPage.test.jsx @@ -17,7 +17,7 @@ jest.mock('../product-tours/ProductTours', () => function () { }); describe('Loaded Tab Page', () => { - const mockData = { activeTabSlug: 'courseware', metadataModel: 'coursewareMeta' }; + const mockData = { activeTabSlug: 'courseware' }; beforeAll(async () => { const store = await initializeTestStore({ excludeFetchSequence: true }); diff --git a/src/tab-page/LoadedTabPage.tsx b/src/tab-page/LoadedTabPage.tsx index f1c3e8271f..dd87ef28e1 100644 --- a/src/tab-page/LoadedTabPage.tsx +++ b/src/tab-page/LoadedTabPage.tsx @@ -19,7 +19,6 @@ interface LoadedTabPageProps { activeTabSlug: string; children?: React.ReactNode; courseId: string; - metadataModel: string; unitId?: string | null; } @@ -27,7 +26,6 @@ const LoadedTabPage = ({ activeTabSlug, children = null, courseId, - metadataModel, unitId = null, }: LoadedTabPageProps) => { const { @@ -72,7 +70,6 @@ const LoadedTabPage = ({ )} { activeTabSlug={tab} courseId={courseId} courseStatus={courseStatus} - metadataModel={`${slice}Meta`} > {children} diff --git a/src/tab-page/TabPage.test.jsx b/src/tab-page/TabPage.test.jsx index f4a72c8f68..049e226694 100644 --- a/src/tab-page/TabPage.test.jsx +++ b/src/tab-page/TabPage.test.jsx @@ -11,6 +11,10 @@ jest.mock('./LoadedTabPage', () => function () { return
; }); +jest.mock('../product-tours/newUserCourseHomeTour/LaunchCourseHomeTourButton', () => function () { + return
; +}); + jest.mock('../generic/ToastContext', () => ({ ...jest.requireActual('../generic/ToastContext'), useToast: jest.fn(), @@ -98,6 +102,16 @@ describe('Tab Page', () => { expect(screen.getByTestId('LoadedTabPage')).toBeInTheDocument(); }); + it('renders the screen-reader tour button on the outline tab', () => { + render(, { wrapWithRouter: true }); + expect(screen.getByTestId('sr-tour-button')).toBeInTheDocument(); + }); + + it('does not render the tour button on other tabs', () => { + render(, { wrapWithRouter: true }); + expect(screen.queryByTestId('sr-tour-button')).not.toBeInTheDocument(); + }); + describe('React Query courseStatus', () => { const metaWithAccess = { data: { courseAccess: { hasAccess: true } } }; diff --git a/src/tab-page/TabPage.tsx b/src/tab-page/TabPage.tsx index ead0375972..22064f3a13 100644 --- a/src/tab-page/TabPage.tsx +++ b/src/tab-page/TabPage.tsx @@ -34,7 +34,6 @@ export interface TabPageProps { activeTabSlug: string; courseId?: string; courseStatus: CourseStatus; - metadataModel: string; unitId?: string; children?: ReactNode; } @@ -71,7 +70,6 @@ const TabPage = ({ activeTabSlug, courseId, courseStatus, - metadataModel, unitId, children, }: TabPageProps) => { @@ -116,8 +114,12 @@ const TabPage = ({ ); - const renderTourButton = () => { - if (metadataModel !== 'courseHomeMeta') { return null; } + // The outline page renders a visible "launch tour" button deep in the DOM; no other tab + // renders it. For screen-reader users we render a screen-reader-only copy above the header, + // where it won't be buried (a11y rationale: + // https://github.com/openedx/frontend-app-learning/pull/750#discussion_r755536879). + const renderSrOnlyTourButton = () => { + if (activeTabSlug !== 'outline') { return null; } return (); }; @@ -131,7 +133,6 @@ const TabPage = ({ {children} @@ -148,7 +149,7 @@ const TabPage = ({ return ( {shouldRenderContent && renderToast()} - {shouldRenderContent && renderTourButton()} + {shouldRenderContent && renderSrOnlyTourButton()} {isLoading && renderLoading()} {shouldRenderContent && renderLoadedTabPage()}