Skip to content

Add support for dynamically displaying the How You'll Learn cards#3198

Merged
jkachel merged 13 commits into
mainfrom
jkachel/10856-toggle-hyl-cards
Apr 28, 2026
Merged

Add support for dynamically displaying the How You'll Learn cards#3198
jkachel merged 13 commits into
mainfrom
jkachel/10856-toggle-hyl-cards

Conversation

@jkachel
Copy link
Copy Markdown
Contributor

@jkachel jkachel commented Apr 14, 2026

What are the relevant tickets?

Closes mitodl/hq#10856
Depends on mitodl/mitxonline#3487

Description (What does it do?)

The dependent PR adds a handful of fields to the product pages that allow the cards in the How You'll Learn section to be toggled on and off. This is the other side of that; it updates the product pages to only display the cards that are checked in the CMS page.

Screenshots (if appropriate):

(Probably worth noting here that I didn't change any of the text on the 4 cards that exist. So, "Learn by Doing" seemed like it should match up with the "Practical Application" card, so that's what I set it to.)

Screenshot 2026-04-14 at 11 57 44 AM Screenshot 2026-04-14 at 11 57 13 AM

How can this be tested?

Test with both a course page and a program page. (Ideally, also test with a program-course; those don't have a separate product page necessarily but good to check anyway.)

For the given item, go into the CMS in MITx Online and check random boxes in the How You'll Learn section and publish your changes. Then, check the product page in Learn. You should only see cards for the selected items.

Additional Context

We'll want to additionally customize this according to the kind of course or provider (or maybe some other thing). The wording and iconography may need to change for different situations - UAI versus xPRO versus MITx, etc. - which we can do in the frontend. This PR doesn't implement that, though.

This needs some text and icons for the new cards. We had these before I started mucking with it:

  • Practical Application - "Learn by Doing"
  • Real-World Learning
  • AI-Enabled Support
  • Stackable Credentials

The new ones are "Learn From Others" and "Learn On Demand". They have the text from the issue but I reused the brains icon.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 14, 2026

OpenAPI Changes

2 changes: 0 error, 1 warning, 1 info

View full changelog

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

@jkachel jkachel marked this pull request as ready for review April 15, 2026 15:36
Copilot AI review requested due to automatic review settings April 15, 2026 15:36
@jkachel jkachel added the Needs Review An open Pull Request that is ready for review label Apr 15, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates product pages to dynamically render the “How you’ll learn” cards based on CMS-controlled boolean fields introduced in a dependent API PR.

Changes:

  • Replaces static “How you’ll learn” placeholder data with a fixed option set filtered by CMS booleans on the page object.
  • Updates Course/Program product pages to pass page into HowYoullLearnSection.
  • Extends page test factories with the new hyl_choice_* boolean fields.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
frontends/main/src/app-pages/ProductPages/ProgramPage.tsx Passes page to HowYoullLearnSection instead of default placeholder cards.
frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.tsx Same as ProgramPage: switches HowYoullLearnSection to be page-driven.
frontends/main/src/app-pages/ProductPages/CoursePage.tsx Same as above for course product pages.
frontends/main/src/app-pages/ProductPages/HowYoullLearnSection.tsx Introduces option registry and filters cards based on hyl_choice_* fields on the page.
frontends/api/src/mitxonline/test-utils/factories/pages.ts Adds defaults for the new hyl_choice_* booleans in test factories.

Comment on lines +141 to +145
const filteredOptions = HOW_YOULL_LEARN_OPTIONS.filter(
(option) =>
page.hasOwnProperty(option.name) &&
(page as unknown as Record<string, boolean>)[option.name],
)
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

Avoid calling page.hasOwnProperty(...) directly and the unknown as Record<string, boolean> cast. Prefer Object.prototype.hasOwnProperty.call(page, option.name) (or option.name in page) and update the name type to a keyof of the page types so TypeScript can enforce that option names match real API fields.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Done in commit 7dc5e6a. Changes made:

  • Removed page.hasOwnProperty(option.name) entirely — key existence is guaranteed by the typed HylChoiceKey union
  • Replaced unknown as Record<string, boolean> with Record<HylChoiceKey, boolean | undefined> (no unknown intermediary, plus an explicit === true check to safely handle any missing/undefined fields)
  • Defined HylChoiceKey as an explicit string union of the known hyl_choice_* field names, typed into HowYoullLearnOption.name — a comment notes it should be replaced with Extract<keyof CoursePageItem & keyof ProgramPageItem, \hyl_choice_${string}`>` once the dependent API PR is published

Also fixed the array-index React key (addressed by the adjacent review comment): the intermediate items array was removed and the render now maps directly over filteredOptions, keying each item on option.name.

Comment on lines 153 to 154
{items.map((item, index) => (
<HowYoullLearnItem key={index}>
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

Using the array index as a React key can cause incorrect reconciliation when cards are toggled on/off (items shift and React may reuse DOM/state for the wrong card). Use a stable key derived from the option (e.g., the hyl_choice_* name) by mapping filteredOptions directly and keying on option.name.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Comment thread frontends/main/src/app-pages/ProductPages/HowYoullLearnSection.tsx Outdated
@jkachel
Copy link
Copy Markdown
Contributor Author

jkachel commented Apr 15, 2026

This is ready for review but won't be mergeable until the MITx Online PR gets merged.

@jkachel
Copy link
Copy Markdown
Contributor Author

jkachel commented Apr 16, 2026

32deed8 requires a MITx Online instance that's at commit mitodl/mitxonline@8d0b1df to work - the HYL API data was refactored significantly.

Copy link
Copy Markdown
Contributor

@ChristopherChudzicki ChristopherChudzicki left a comment

Choose a reason for hiding this comment

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

Tested this with your branch client via mitodl/mitxonline-api-clients#63... API data comes through well.

If you have time, i think it would be nice to

  • change the factory to faker / lorem data
  • add a test asserting it renders whichever ones the backend is returning
    • this is complicated a bit by the icon values, I guess.

but it's a very simple component so not too worried about it.

Comment thread frontends/main/src/app-pages/ProductPages/HowYoullLearnSection.tsx Outdated
Comment on lines +97 to +99
default:
console.warn(`Unknown how_youll_learn icon: ${src}`)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should flag in the issue or somewhere that right now we have 3 repeated icons if you enable all of them:

Image

Originally there was a fifth icon (we only had 2 duplicates) https://github.com/mitodl/mit-learn/blob/cdabbf911573599118eeb4766ffabd2035a659d0/frontends/main/public/images/product/icon_book_play.png but I removed it when we culled the hard-coded list down to 4.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The "Learn From Others" and "Learn On Demand" ones are the new ones so will ping folks about that - initially i think we'll probably just not use those two. (I need to get something in place to set the defaults for existing courses, too; we discussed this in the MITx Online PR and then I forgot to put it in..)

Copy link
Copy Markdown
Contributor

@ChristopherChudzicki ChristopherChudzicki left a comment

Choose a reason for hiding this comment

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

errr requesting a change more as a "don't forget" but

Requested change: update the api client after your mitxonline pr gets to production

looks fine otherwise, with minor comments earlier.

@jkachel jkachel force-pushed the jkachel/10856-toggle-hyl-cards branch from 32deed8 to 8fcd872 Compare April 23, 2026 17:41
@jkachel
Copy link
Copy Markdown
Contributor Author

jkachel commented Apr 23, 2026

@ChristopherChudzicki the API client is updated and I think this ended up triggering a handful of other, minor changes because some types changed in the client.. if you want to give it another quick run through please do - otherwise i will plan to merge this sometime tomorrow (Friday) in the afternoon.

@jkachel jkachel merged commit 421d258 into main Apr 28, 2026
13 checks passed
@jkachel jkachel deleted the jkachel/10856-toggle-hyl-cards branch April 28, 2026 20:07
@odlbot odlbot mentioned this pull request Apr 29, 2026
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review An open Pull Request that is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants