Skip to content

docs: ADR and migration guidance for per-page document titles#252

Merged
arbrandes merged 2 commits intoopenedx:mainfrom
arbrandes:arbrandes/page-titles
Apr 29, 2026
Merged

docs: ADR and migration guidance for per-page document titles#252
arbrandes merged 2 commits intoopenedx:mainfrom
arbrandes:arbrandes/page-titles

Conversation

@arbrandes
Copy link
Copy Markdown
Contributor

@arbrandes arbrandes commented Apr 29, 2026

Description

Refs #250.

Adds ADR 0015 establishing that each route-level page component in a frontend-base app owns the document title via <Helmet> from react-helmet, with the i18n message pattern {page}.page.title defaulting to {Page Name} | {siteName}. This is the same pattern frontend-app-authn already follows.

The motivation is straightforward. A frontend-base site renders all of its apps under a single index.html, so the document title only changes if a page sets it. Authn does; learner-dashboard and instructor-dashboard don't. After signing in, the browser tab keeps reading "Login | " while the user sits on a different app, which is a UX regression.

The migration how-to gains a corresponding section so apps being ported don't ship without titles. Follow-up PRs will add the pattern to the apps that are missing it.

LLM usage notice

Built with assistance from Claude.

Add ADR 0015 establishing that route-level page components own the
document title via <Helmet>, and a corresponding section in the
migration how-to so apps being ported don't ship without titles.

Co-Authored-By: Claude <noreply@anthropic.com>
@arbrandes arbrandes linked an issue Apr 29, 2026 that may be closed by this pull request
Show the example as src/Main.jsx (the component the route lazy-loads),
not a generic content component, since that's where the apps actually
add Helmet.  Note the nested-routes case and the don't-reuse-an-h1
gotcha that came up in both learner-dashboard and instructor-dashboard.

Co-Authored-By: Claude <noreply@anthropic.com>
@brian-smith-tcril
Copy link
Copy Markdown
Contributor

Overall I think this makes sense, my only concern is that helmet always "wins," so authenticated apps must use a helmet title even if they just want the basic fallback because the authn title already "won."

I know you mentioned

frontend-base does not own the title and does not re-export Helmet.

but you followed that with (emphasis mine)

Each app that needs to set a title declares react-helmet as a dependency.

the current ADR means that every app "needs to set a title."

I think it's worth looking into having frontend-base set a "true fallback." I think this is likely possible with react-helmet's defaultTitle, but I haven't tested it out.

@arbrandes
Copy link
Copy Markdown
Contributor Author

arbrandes commented Apr 29, 2026

Not worth it. Every app needs to set a title, period.

@arbrandes
Copy link
Copy Markdown
Contributor Author

(Every app with a top-level route, I mean.)

Copy link
Copy Markdown
Contributor

@brian-smith-tcril brian-smith-tcril left a comment

Choose a reason for hiding this comment

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

With

Every app needs to set a title, period.

in mind, this LGTM.

I do think it's worth digging into

If many more apps adopt the pattern and a
wrapper proves valuable, we can revisit.

and possibly a way to enforce the every app with a top level route needs a title rule, but those are far into follow-up territory.

@bradenmacdonald
Copy link
Copy Markdown
Contributor

Should we consider the React 19 upgrade? You can still use react-helmet with React 19, but it now supports just including <title> directly which is way simpler.

@arbrandes arbrandes merged commit 1f9f07d into openedx:main Apr 29, 2026
5 checks passed
@arbrandes arbrandes deleted the arbrandes/page-titles branch April 29, 2026 16:54
@arbrandes
Copy link
Copy Markdown
Contributor Author

@bradenmacdonald

it now supports just including <title> directly

Well, I'll be damned. That'll be awesome. Let's totally do that once we upgrade to React 19. 👍🏼

@openedx-semantic-release-bot
Copy link
Copy Markdown

🎉 This PR is included in version 1.0.0-alpha.43 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Page title not updating after logging in

4 participants