feat: [FC-86] Updated Header links #24

PKulkoRaccoonGang · 2025-09-16T15:39:05Z

Note

Dependencies:

feat [FC-86]: update course_about & catalog link generation edx-platform#37342
feat [FC-86]: add redirects to mfe for catalog-related legacy pages edx-platform#37341

Note

Related PR:

docs: [FC-86] Restructure Modify Items section with Add/Replace examples frontend-component-header#598

Description

This PR extends the frontend-component-header settings for the Index, Course Catalog, and Course About pages.

Useful information

Figma design
For the MFE Catalog pages we can use a similar approach to frontend-app-learner-dashboard with passing the necessary links directly to the frontend-component-header.
Slack conversation
[DEPR]: Deprecation of Legacy Home, Course Catalog, and Course About Pages in Favor of MFE Implementations edx-platform#36785

Initial setup

Clone and install the tutor-mfe plugin from the draft PR that adds support for Catalog MFE.
The edx-platform should have changes from these PRs feat [FC-86]: update course_about & catalog link generation edx-platform#37342 and feat [FC-86]: add redirects to mfe for catalog-related legacy pages edx-platform#37341. Alternatively, for testing, you can switch to the rg-tick/develop testing branch, which already contains these commits.

How Has This Been Tested?

Header for unauthenticated users

The header must include the following links:

Logo
- /learner-dashboard - link for Home, Course about and Catalog pages. If the user is not authenticated, redirect to the login page.
Explore courses (/courses) - visible only if ENABLE_COURSE_DISCOVERY is enabled (Django Site Configuration)
Help - visible only if SUPPORT_URL is configured (Django Site Configuration)
Sign Up button
Login button

Header for authenticated users

The header must include the following links:

Logo
- /learner-dashboard - Home page, Course About and Catalog pages
Courses (/learner-dashboard)
Programs (/dashboard/programs) - visible only if ENABLE_PROGRAMS is enabled (Django Site Configuration)
Discover new (/courses) - visible only if NON_BROWSABLE_COURSES is enabled (Django Site Configuration)
Help - visible only if SUPPORT_URL is configured (Django Site Configuration)

Acceptance Criteria

Depending on the value of ENABLE_COURSE_DISCOVERY, legacy links should redirect to the corresponding new MFE pages.
The header links must match the list provided above for both authenticated and unauthenticated users.
On mobile devices, the logic should be identical to the desktop functionality.

openedx-webhooks · 2025-09-16T15:39:10Z

Thanks for the pull request, @PKulkoRaccoonGang!

This repository is currently maintained by @openedx/committers-frontend.

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

🔘 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.

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.

codecov · 2025-09-24T08:10:26Z

Codecov Report

❌ Patch coverage is 97.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.96%. Comparing base (1cf6194) to head (95ec797).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/header/hooks/useMenuItems.ts	96.15%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #24      +/-   ##
==========================================
+ Coverage   96.90%   96.96%   +0.05%     
==========================================
  Files          42       46       +4     
  Lines         323      362      +39     
  Branches       51       60       +9     
==========================================
+ Hits          313      351      +38     
- Misses         10       11       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

src/header/CatalogHeader.tsx

PKulkoRaccoonGang · 2025-09-24T10:59:33Z

src/utils.ts

[inform]: Moved from frontend-app-learner-dashboard

Copying this in here seems like a good short term solution, but if this is a common pattern we're using across MFEs it likely makes sense to add it to frontend-platform. It'd be good to make a follow-up issue on frontend-platform linking to the implementation here and in frontend-app-learner-dashboard.

Created new issue: openedx/frontend-platform#835

ihor-romaniuk · 2025-09-24T12:42:06Z

.env.test

 HOMEPAGE_PROMO_VIDEO_YOUTUBE_ID='test-youtube-id'
 ENABLE_COURSE_DISCOVERY=true
 ENABLE_PROGRAMS=true
+SUPPORT_URL='https://support.example.com'


[question] Let's add an empty SUPPORT_URL to prod and dev env files

Added to 3559b0e#diff-e9cbb0224c4a3d23a6019ba557e0cd568c1ad5e1582ff1e335fb7d99b7a1055dR26

ihor-romaniuk · 2025-09-24T12:47:51Z

src/header/utils.ts

+  }
+  return getConfig().LMS_BASE_URL;
+};


Suggested change

}

return getConfig().LMS_BASE_URL;

};

}

return getConfig().LMS_BASE_URL;

};

Added to 3559b0e#diff-e9cbb0224c4a3d23a6019ba557e0cd568c1ad5e1582ff1e335fb7d99b7a1055dR26

brian-smith-tcril

Thank you for this!

This is just a first pass.

I noticed isNotHomePage and was a bit confused about it, which led to a bigger "how should the logo link behave" question.

Any thoughts would be great!

src/header/CatalogHeader.tsx

src/header/utils.ts

PKulkoRaccoonGang · 2025-09-24T18:54:11Z

@brian-smith-tcril thanks!

I’ve added more context about the logo link on the home page in this comment. It would be great to hear your thoughts on it 💯

brian-smith-tcril

Overall this looks really good!

I left a couple comments with questions in there, but nothing super major.

Thanks again for your work on this!

src/header/hooks/useMenuItems.ts

brian-smith-tcril · 2025-09-25T16:25:35Z

src/header/CatalogHeader.test.tsx

+jest.mock('@edx/frontend-component-header', () => ({
+  __esModule: true,
+  default: jest.fn(({ mainMenuItems, logoDestination, secondaryMenuItems }) => (
+    <div data-testid="header">
+      <div data-testid="main-menu">{JSON.stringify(mainMenuItems)}</div>
+      <div data-testid="logo-destination">{logoDestination}</div>
+      <div data-testid="secondary-menu">{JSON.stringify(secondaryMenuItems)}</div>
+    </div>
+  )),
+}));


Does this need to be mocked for this to be testable?

My worry here is that by mocking this the tests won't catch any issues stemming from updates to frontend-component-header.

Does this need to be mocked for this to be testable?

Not necessarily. I think you’re right 💯
It would be better to add tests that directly cover the header component.

src/header/messages.ts

brian-smith-tcril · 2025-09-25T16:30:28Z

src/utils.ts

Copying this in here seems like a good short term solution, but if this is a common pattern we're using across MFEs it likely makes sense to add it to frontend-platform. It'd be good to make a follow-up issue on frontend-platform linking to the implementation here and in frontend-app-learner-dashboard.

src/header/CatalogHeader.test.tsx

sarina · 2025-10-01T16:25:14Z

Can this be re-reviewed now that the edx-platform changes have merged?

brian-smith-tcril

Overall this looks good!

I left a couple comments with questions about some of the tests. I'm also going to pull this down and verify it works against latest master of edx-platform.

src/header/hooks/useMenuItems.test.tsx

brian-smith-tcril · 2025-10-02T14:21:38Z

Local testing looks good overall, I do have a question though:

Note

Based on the Slack conversation we can proceed without changes to frontend-component-header (no need to make a follow up issue).

I don't know of a scenario before this MFE that leads to seeing the header while logged out. The log in buttons differ from what appears on the legacy page.

vs

Specifically:

Wording changed
- "Sign in" -> "Login"
- "Register for free" -> "Sign Up"
Ordering changed
- Legacy has:
  - "Sign in" as primary? brand? to the right
  - "Register for free" as outline-primary? outline-brand? to the left
- MFE header has:
  - "Sign up" as outline-primary to the right
  - "Login" as link to the left

This is coming from LoggedOutItems in frontend-component-header, so it's not something that should block this PR, but I wanted to flag it here.

PKulkoRaccoonGang · 2025-10-03T08:39:46Z

This is coming from LoggedOutItems in frontend-component-header, so it's not something that should block this PR, but I wanted to flag it here.

I noticed that in the Learning MFE, the header displayed for unauthenticated users looks very similar to what we currently have in the legacy version.

The Register and Sign in buttons are rendered by the AnonymousUserMenu component, which is included within the LearningHeader. In other words, this is technically the header version used for the Learning MFE.

I think it might be a good idea to reuse the LearningHeader and make some of its elements more generic, so they could serve as the basis for a universal header for the LMS part of the platform. However, these changes would introduce breaking changes, and I’m not sure we want to take that on in this iteration.

If this makes sense to you, I’d be happy to create an issue for it.

brian-smith-tcril · 2025-10-03T13:26:51Z

@PKulkoRaccoonGang re: links in the LearningHeader

Thanks for doing that discovery! I don't think trying to reuse LearningHeader within frontend-component-header is something we should focus on with frontend-base work underway.

I do, however, think having consistency in our "Register"/"Sign Up"/"Login"/"Sign in" links for all the headers provided by frontend-component-header is worth making an issue on frontend-component-header for.

Any work on that definitely falls outside the scope of this FC, especially considering the outcome of yesterday's slack conversation about this being "they are fine as-is."

brian-smith-tcril

LGTM!

After a rebase this will be ready to merge!

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Sep 16, 2025

openedx-webhooks added this to Contributions Sep 16, 2025

github-project-automation bot moved this to Needs Triage in Contributions Sep 16, 2025

PKulkoRaccoonGang self-assigned this Sep 16, 2025

mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Sep 16, 2025

mphilbrick211 moved this from Needs Triage to Waiting on Author in Contributions Sep 16, 2025

PKulkoRaccoonGang force-pushed the Peter_Kulko/header-links branch from 5d4be3a to da5875c Compare September 24, 2025 08:42

PKulkoRaccoonGang commented Sep 24, 2025

View reviewed changes

src/header/CatalogHeader.tsx Outdated Show resolved Hide resolved

PKulkoRaccoonGang force-pushed the Peter_Kulko/header-links branch from 2b4a7a3 to 2a18e95 Compare September 24, 2025 10:58

PKulkoRaccoonGang commented Sep 24, 2025

View reviewed changes

PKulkoRaccoonGang force-pushed the Peter_Kulko/header-links branch from 2a18e95 to 4074ae4 Compare September 24, 2025 11:04

PKulkoRaccoonGang mentioned this pull request Sep 24, 2025

feat: [FC-86] Updated Header links raccoongang/frontend-app-catalog#18

Closed

PKulkoRaccoonGang marked this pull request as ready for review September 24, 2025 11:51

PKulkoRaccoonGang changed the title ~~DRAFT: [FC-86] Updated Header links~~ feat: [FC-86] Updated Header links Sep 24, 2025

PKulkoRaccoonGang mentioned this pull request Sep 24, 2025

docs: [FC-86] Restructure Modify Items section with Add/Replace examples openedx/frontend-component-header#598

Merged

5 tasks

ihor-romaniuk reviewed Sep 24, 2025

View reviewed changes

PKulkoRaccoonGang closed this Sep 24, 2025

github-project-automation bot moved this from Waiting on Author to Done in Contributions Sep 24, 2025

PKulkoRaccoonGang reopened this Sep 24, 2025

PKulkoRaccoonGang requested a review from brian-smith-tcril September 24, 2025 14:06

brian-smith-tcril reviewed Sep 24, 2025

View reviewed changes

src/header/CatalogHeader.tsx Outdated Show resolved Hide resolved

src/header/utils.ts Outdated Show resolved Hide resolved

PKulkoRaccoonGang requested a review from brian-smith-tcril September 24, 2025 18:54

PKulkoRaccoonGang force-pushed the Peter_Kulko/header-links branch from 2fb6bf8 to 624f186 Compare September 25, 2025 08:59

ihor-romaniuk approved these changes Sep 25, 2025

View reviewed changes

brian-smith-tcril reviewed Sep 25, 2025

View reviewed changes

PKulkoRaccoonGang requested a review from brian-smith-tcril September 26, 2025 10:44

brian-smith-tcril reviewed Sep 26, 2025

View reviewed changes

src/header/CatalogHeader.test.tsx Show resolved Hide resolved

PKulkoRaccoonGang mentioned this pull request Sep 29, 2025

Inconsistent logic between ENABLE_COURSE_DISCOVERY and NON_BROWSABLE_COURSES #25

Open

brian-smith-tcril reviewed Oct 2, 2025

View reviewed changes

feat: updated Header links

7e88d0c

brian-smith-tcril approved these changes Oct 3, 2025

View reviewed changes

PKulkoRaccoonGang added 9 commits October 3, 2025 16:37

refactor: some refactoring

cda6af9

refactor: code refactoring

36660d8

refactor: added some tests

44ae8b5

refactor: after review

3b8b761

refactor: removed logo url logic

5531de1

refactor: after review

3ae7136

refactor: corrected header tests

50e8925

refactor: corrected tests

afa6c70

refactor: after rebase

95ec797

PKulkoRaccoonGang force-pushed the Peter_Kulko/header-links branch from 44158dc to 95ec797 Compare October 3, 2025 13:39

brian-smith-tcril merged commit 8953eb6 into openedx:master Oct 3, 2025
6 checks passed

feat: [FC-86] Updated Header links #24

feat: [FC-86] Updated Header links #24

Uh oh!

Conversation

PKulkoRaccoonGang commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Useful information

Initial setup

How Has This Been Tested?

Header for unauthenticated users

Header for authenticated users

Uh oh!

openedx-webhooks commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brian-smith-tcril left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

PKulkoRaccoonGang commented Sep 24, 2025

Uh oh!

brian-smith-tcril left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sarina commented Oct 1, 2025

Uh oh!

brian-smith-tcril left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brian-smith-tcril commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PKulkoRaccoonGang commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brian-smith-tcril commented Oct 3, 2025

Uh oh!

brian-smith-tcril left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

PKulkoRaccoonGang commented Sep 16, 2025 •

edited

Loading

openedx-webhooks commented Sep 16, 2025 •

edited

Loading

codecov bot commented Sep 24, 2025 •

edited

Loading

brian-smith-tcril commented Oct 2, 2025 •

edited

Loading

PKulkoRaccoonGang commented Oct 3, 2025 •

edited

Loading