fix(mitxonline): align the instructor dashboard header with the learning MFE - #224
Open
Anas12091101 wants to merge 6 commits into
Open
fix(mitxonline): align the instructor dashboard header with the learning MFE#224Anas12091101 wants to merge 6 commits into
Anas12091101 wants to merge 6 commits into
Conversation
…ing MFE The instructor dashboard is served by the OEP-65 Site Project, whose header is frontend-base's rather than the learning MFE's, so four things the learning header does were missing. UAI / MIT Learn courses now show the course title alone. Their keys carry no public org + course number, and the learning header hides that line for them (addLearningCourseInfoSlotOverride in legacy/mitxonline/common-mfe-config.env.jsx). This REPLACEs the course info lockup that @openedx/frontend-app-instructor-dashboard appends to the header's primaryLinks slot; the widget id it registers is `...slotShowcase.headerLink`, an upstream copy-paste artifact and the only handle on that widget, so it needs re-checking when that package is bumped. The Dashboard user-menu item is now limited to viewports at or below 991px. The standalone Dashboard button sits right next to the menu above that width, so the item was a duplicate; the SCSS hides the button at 991px, which is where legacy switched the item on (`includeDashboard: isMobile()`). Two styling gaps came out of comparing the two headers side by side with both dev servers running: The course title rendered at weight 700 against the learning header's 400. Legacy neutralises Bootstrap's `font-weight-bold` with a `!important` rule that wins on specificity; frontend-base loads Paragon into a cascade layer, where a layered site rule cannot outrank an `!important` utility. Since we own the component, it simply stops emitting the class. The logo-to-title gap was 16px against the learning header's 26px, and long titles ran on instead of truncating. Both come from stylesheets outside this repo that the Site Project never had: frontend-app-learning's own src/index.scss (`header .logo`, contributing 16px of the gap) and frontend-component-header's `.learning-header .course-title-lockup span`. Ported into mitxonline.scss, with frontend-base's `Nav.ml-3` wrapper replaced by the legacy lockup element so those rules apply at all. Every box in both headers now measures identically at 1440px -- logo, lockup, org line and title position -- for a UAI course and a non-UAI one. The footer's missing MIT logo is a runtime-config gap, not a code one: commonAppConfig.mitolFooter.footerLogoUrl is unset, so the footer falls back to headerLogoImageUrl, whose SVG is drawn fill="white" for the dark header and is invisible on the light footer. Fixed in mitodl/ol-infrastructure#4893. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The README incorrectly describes configuration from an unmerged infrastructure PR as already present.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Aligns the MITx Online instructor-dashboard header with the learning MFE.
Changes:
- Adds responsive Dashboard menu behavior and UAI title-only course information.
- Matches learning-header spacing, weight, and title truncation.
- Updates frontend configuration documentation.
File summaries
| File | Description |
|---|---|
shared/src/styles/mitxonline.scss |
Aligns header spacing and truncation. |
shared/src/header/index.tsx |
Adds course lockup and responsive menu logic. |
frontend/README.md |
Documents header behavior and runtime configuration. |
mitxonline/README.md |
Updates migration status. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+152
to
+153
| All of them are set in ol-infrastructure; the two below were added late, so an | ||
| environment that has not been applied since will still fall back: |
Contributor
Author
There was a problem hiding this comment.
Correct, thanks — fixed in 533ca92. The paragraph now says the keys are not in ol-infrastructure at all yet, names the open PR that adds them, and treats the merge as a prerequisite alongside the apply.
Contributor
Author
There was a problem hiding this comment.
Resolved differently in the end (c5fb9bd). Naming the open PR was accurate today but stops being true once it merges, and this README documents the configuration contract rather than PR state. It now just says each key has a fallback and what that fallback gets you, which reads the same before and after the merge. Which PR carries the infra change stays in this PR description.
… set Addresses review feedback on #224. The README claimed the mitolHeader and footerLogo keys were all set in ol-infrastructure and that only an un-applied stack would fall back. They are not there at all yet -- ol-infrastructure#4893 adds them and is still open -- so every environment falls back today, and the merge is as much a prerequisite as the apply. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comment claimed UAI / MIT Learn courses "are published without a public org + course number (their keys are internal)". Nothing says that. The legacy override and the slot table in legacy/Readme.md both record the behaviour and not the reason, and the override arrived in a bulk slot-config import (7eb5519) with no explanation. The claim in f70d63f's commit message is wrong for the same reason. It is also contradicted by the data: course-v1:UAI_SOURCE+UAI111+UAI_11 reports org UAI_SOURCE and number UAI111, and the instructor dashboard rendered both before this branch. So the values exist and hiding them is a product decision, which is what the comment now says -- along with a note to check with the team rather than inferring a rule from it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
They had grown into essays -- 107 added comment lines for ~90 lines of code. Trimmed to 51, keeping only what the code does not say for itself: the widget id that looks like a mistake, the 991px breakpoint the SCSS shares, the shared react-query key, and the two cascade-layer workarounds. The UAI rule is now just that -- we hide the course number on UAI courses, as the learning header does. Comments only; the header still measures identically against the learning MFE. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
533ca92 traded one wrong statement for one with a shelf life: it said the keys are missing from ol-infrastructure and named the open PR, which stops being true the moment that merges. The README documents the configuration contract, so it should read the same before and after. Now it just says each key has a fallback and what that fallback gets you -- useful next to "New deployments must include the equivalent block there", and true either way. Which PR carries the change belongs in the PR description, where it already is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
b19cd48 removed "they have no public org + number" from the code comment but missed this bullet, which carried the same claim. It is not recorded anywhere and the values do exist, so the bullet now just states the rule. Trimmed while there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ardiea
approved these changes
Sep 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
https://github.com/mitodl/hq/issues/12777
Description
The instructor dashboard is served by the OEP-65 Site Project, so it gets frontend-base's header rather than the learning MFE's. This fixes the four items in the issue, plus two styling gaps found while comparing the two headers side by side.
What changed
Dashboardin the user menuEvery box in both headers — logo, lockup, org line, title position — now measures identically at 1440px, for a UAI course and a non-UAI one.
Two notes on why the code looks the way it does:
font-weight-boldinstead of overriding it. Legacy neutralises that utility with an!importantrule that wins on specificity; frontend-base loads Paragon into a cascade layer, where a layered site rule cannot outrank an!importantutility.frontend-app-learning/src/index.scss(header .logo, which contributes 16px of the 26px gap) andfrontend-component-header's.learning-header .course-title-lockup span. Both are ported intomitxonline.scss, and frontend-base'sNav.ml-3wrapper is replaced by the legacy lockup element so they apply at all.The footer MIT logo is not fixed here
It is a runtime-config gap, not code.
commonAppConfig.mitolFooter.footerLogoUrlis unset, so the footer falls back toheaderLogoImageUrl— an SVG drawnfill="white"for the dark header, invisible on the light footer. Fixed in mitodl/ol-infrastructure#4893.How to test
Needs a local LMS with
ENABLE_MFE_CONFIG_API = Trueand a populatedFRONTEND_SITE_CONFIG(commonAppConfig.mitolHeader+mitolFooter)./apps/instructor-dashboard/<courseId>/course_infoon the dev server.course-v1:UAI_) → the header shows the course title only, noORG NUMBERline above it.ORG NUMBERstill shows above the title.Dashboarditem (the Dashboard button beside the menu covers it). Narrow the window below 992px → the button disappears and theDashboarditem appears./learn/course/<courseId>/home) → title weight, logo gap and vertical position should match.Also check
course-v1:MITxT+CTL.SCx_WM+1T2026, which is UAI by explicit exception, behaves like a UAI course.