Skip to content

refactor: set the page title with Helmet on seven pages - #3345

Open
PRAYAG-PATEL-1998 wants to merge 7 commits into
openedx:masterfrom
PRAYAG-PATEL-1998:fix/897-helmet-document-title
Open

PRAYAG-PATEL-1998 wants to merge 7 commits into
openedx:masterfrom
PRAYAG-PATEL-1998:fix/897-helmet-document-title

Conversation

@PRAYAG-PATEL-1998

Copy link
Copy Markdown

Description

Seven pages set the browser tab title with document.title = …. This PR moves each one to <Helmet><title>…</title></Helmet>, the pattern that CourseExportPage, CourseOutline, VideosPage and other pages already use. The title text is unchanged: each page still builds it with getPageHeadTitle and its existing intl heading message.

  • PagesAndResources, CustomPages, FilesPage, ScheduleAndDetails, CourseTeam: delete the document.title line and add a <Helmet> as the first child of the main return.
  • CourseUnit: delete the useEffect that only set the title (and its unused import) and add a <Helmet> in the main return.
  • GroupConfigurations: this page already rendered a <Helmet>, so I only delete the duplicate document.title line.
  • Each page gets one test, sets the page title through Helmet, in its existing test file. It reads Helmet.peek().title, as CourseExportPage.test.tsx does.

Users affected: Course Authors (the browser tab title only). I built each title from the same helper and messages as before, and I checked the tab titles in a running Studio.

Supporting information

Closes #897. Maintainer direction in the issue: "Please open one PR for all seven and anything else related to the titles." To find anything else related, I searched for other direct title writes on this branch:

$ git grep -n -E "document\.title|querySelector\('title'\)" -- src plugins ':!*.test.*'
$ echo $?
1

No matches remain. Certificates already sets its title through Helmet (useCertificates returns the title and Certificates.tsx renders it in <title>).

One change from my first comment on the issue: I said each page would get a new translatable title message. That is not needed, because all seven pages already format their title from an existing intl heading message, so this PR adds no messages.

Testing instructions

Tests:

  1. npm ci
  2. TZ=UTC npx fedx-scripts jest --silent src/pages-and-resources src/group-configurations src/course-unit/CourseUnit.test src/course-team src/schedule-and-details src/custom-pages src/files-and-videos/files-page
  3. Expected: 48 suites, 435 tests pass. Claude Code and I both ran this and it passed.

Before the change, the six new tests for CourseTeam, CustomPages, PagesAndResources, FilesPage, ScheduleAndDetails and CourseUnit fail with TypeError: Cannot read properties of undefined (reading 'title'), because those pages mount no <Helmet>. To see this, run the new test files against the base commit.

To check in Studio: open each of the seven pages and compare the tab title with the one on master. I did this on this branch and the tab titles were correct.

Other information

  • The GroupConfigurations test passes before and after this change, because that page already had a <Helmet>. It guards the title but does not prove the fix.
  • Checks run: npm run types, oxlint --type-aware --deny-warnings and dprint check on the 14 changed files. All pass.
  • Checks not run: the full npm run lint (on my Windows PC dprint flags about 2,000 files for CRLF line endings), make validate, and the full Jest suite. CI will run them.
  • Question: these pages now set the title only in their main return, like CourseOutline, VideosPage and GradingSettings. The old code also set it while loading and on permission-denied. Should those returns get a title too? I can add it if you want.

AI use: I directed this change and Claude Code implemented it. I decided what to do: this issue, one PR for the seven pages, and Jest tests plus a manual check in Studio. I brought the options and constraints, we evaluated them together, I made the decisions, and Claude wrote the code in small commits, one per page. The test approach was mine, a test for each page that fails without the fix, and Claude wrote it and tightened it. I also used it to map the project structure, to find the failing code, and to draw out edge cases. Examples: the loading and permission-denied returns that have no title, the duplicate document.title in GroupConfigurations, and the title useEffect in CourseUnit. Claude Code ran the tests and checks listed here, and I ran the tests myself as well. I read the diff and can explain each change. Commits carry a Co-Authored-By trailer for Claude.

Best Practices Checklist

  • Any new files are using TypeScript (.ts, .tsx). (No new files.)
  • Avoid propTypes and defaultProps in any new or modified code.
  • Tests should use the helpers in src/testUtils.tsx (specifically initializeMocks)
  • Do not add new fields to the Redux state/store. Use React Context to share state among multiple components.
  • Use React Query to load data from REST APIs. (Not applicable: no data loading changed.)
  • All new i18n messages in messages.ts files have a description for translators to use. (No new messages.)
  • Avoid using ../ in import paths. To import from parent folders, use @src.

🤖 Generated with Claude Code

PRAYAG-PATEL-1998 and others added 7 commits October 30, 2026 01:38
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… keep Helmet

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @PRAYAG-PATEL-1998!

This repository is currently maintained by @bradenmacdonald.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Submit a signed contributor agreement (CLA)

⚠️ We ask all contributors to the Open edX project to submit a signed contributor agreement or indicate their institutional affiliation.
Please see the CONTRIBUTING file for more information.

If you've signed an agreement in the past, you may need to re-sign.
See The New Home of the Open edX Codebase for details.

Once you've signed the CLA, please allow 1 business day for it to be processed.
After this time, you can re-run the CLA check by adding a comment below that you have signed it.
If the CLA check continues to fail, you can tag the @openedx/cla-problems team in a comment for further assistance.

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Nov 5, 2026
@github-project-automation github-project-automation Bot moved this to Needs Triage in Contributions Nov 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Status: Needs Triage

Development

Successfully merging this pull request may close these issues.

Update document.title to use Helmet

2 participants