Skip to content

feat: [FC-86] Course search for catalog page#42

Merged
brian-smith-tcril merged 10 commits intoopenedx:masterfrom
raccoongang:Peter_Kulko/catalog-data-table-search
Nov 20, 2025
Merged

feat: [FC-86] Course search for catalog page#42
brian-smith-tcril merged 10 commits intoopenedx:masterfrom
raccoongang:Peter_Kulko/catalog-data-table-search

Conversation

@PKulkoRaccoonGang
Copy link
Contributor

@PKulkoRaccoonGang PKulkoRaccoonGang commented Oct 22, 2025

Note

The plugin slots for the course catalog page will be added in the next PR.

Description

Added course search functionality on the course catalog page.

Useful information

Initial setup

  1. Clone and install the tutor-mfe plugin from the draft PR that adds support for Catalog MFE.
  2. Go to http://apps.local.openedx.io:1998/catalog/courses
  3. Change the DEFAULT_PAGE_SIZE value to 2, or create more than 20 courses

How Has This Been Tested?

  1. Go to Course Catalog page (http://apps.local.openedx.io:1998/catalog/courses).
  2. In the search field, type the value you need to find (for example, after I created a course called "Python," typing the course name into the search field displays the "Python" course card).
image
  1. After the search is executed, the faceted sidebar (or filter panel) dynamically updates based on the course data returned by the backend.
  2. When the search input is cleared, the catalog data reverts to its pre-search state.
image
  1. Since the "Python" course is in English, searching for it while filtered by Chinese will yield no results.
image
  1. Search query and active filters operate independently: clearing the filters does not reset the search term.
  2. Entering a search query on the home page redirects the user to the catalog page and displays the corresponding search results.
  3. The visibility/rendering of the course search input is conditional on the ENABLE_COURSE_DISCOVERY setting, which is configured via the Site Configuration.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 22, 2025
@openedx-webhooks
Copy link

openedx-webhooks commented Oct 22, 2025

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.

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.

@github-project-automation github-project-automation bot moved this to Needs Triage in Contributions Oct 22, 2025
@PKulkoRaccoonGang PKulkoRaccoonGang changed the title feat: Course search for catalog page feat: [FC-86] Course search for catalog page Oct 22, 2025
@PKulkoRaccoonGang PKulkoRaccoonGang self-assigned this Oct 22, 2025
@codecov
Copy link

codecov bot commented Oct 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.74%. Comparing base (0c0fc21) to head (b714e69).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #42      +/-   ##
==========================================
+ Coverage   98.57%   98.74%   +0.16%     
==========================================
  Files          89       94       +5     
  Lines         845      953     +108     
  Branches      136      154      +18     
==========================================
+ Hits          833      941     +108     
  Misses         12       12              

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

@mphilbrick211 mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Oct 22, 2025
@mphilbrick211 mphilbrick211 moved this from Needs Triage to Waiting on Author in Contributions Oct 22, 2025
@PKulkoRaccoonGang PKulkoRaccoonGang force-pushed the Peter_Kulko/catalog-data-table-search branch from b737605 to 2769cce Compare October 23, 2025 07:07
@PKulkoRaccoonGang PKulkoRaccoonGang force-pushed the Peter_Kulko/catalog-data-table-search branch 2 times, most recently from 8a179ae to 5850d4d Compare October 23, 2025 08:52
@PKulkoRaccoonGang PKulkoRaccoonGang marked this pull request as ready for review October 23, 2025 08:56
@mphilbrick211 mphilbrick211 moved this from Waiting on Author to Ready for Review in Contributions Oct 29, 2025
@brian-smith-tcril
Copy link
Contributor

I'm trying to understand the functionality leading to the lastSearchQuery logic requirement.

It seems to me the main thing is that if a user searches for something with no results, the search box is cleared but we still display the previous search in the heading

Screencast.From.2025-11-06.12-36-29.mp4

I think my main question here is: do we need to clear the search box? Could we simplify the logic if we just kept the search that led to zero results in the search box?

I also noticed some redirect behavior I don't fully understand the reasoning behind

Screencast.From.2025-11-06.12-41-03.mp4

@mphilbrick211 mphilbrick211 moved this from Ready for Review to In Eng Review in Contributions Nov 6, 2025
@PKulkoRaccoonGang PKulkoRaccoonGang force-pushed the Peter_Kulko/catalog-data-table-search branch 5 times, most recently from 831f3ab to 30d4b63 Compare November 7, 2025 08:50
@PKulkoRaccoonGang
Copy link
Contributor Author

I think my main question here is: do we need to clear the search box? Could we simplify the logic if we just kept the search that led to zero results in the search box?

@brian-smith-tcril thanks! This logic was there to match the behavior in the legacy version. In my latest changes, I tried removing it. I also cleaned up some unnecessary search logic related to the search_string query parameter. I think this refactoring made things a bit cleaner overall, less redundant code and smoother rendering.

Screen.Recording.2025-11-07.at.14.15.01.mov

Copy link
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.

Overall this is looking good!

I left a few comments, including one with a minimal search-on-type implementation in a diff.

I did notice that with search on type the skeleton cards popping in and out feels quite jumpy/sudden. I'd like to do a bit more digging to see if we might be able to defer the displayData in some way too.

https://react.dev/reference/react/useDeferredValue#indicating-that-the-content-is-stale has a nice example that looks quite similar to the backend example from the paragon docs site https://paragon-openedx.netlify.app/components/datatable/#backend-filtering-and-sorting

@brian-smith-tcril
Copy link
Contributor

I made an issue on the Paragon repo to dig into the skeleton card jumpiness (it's an issue for the table view as well, just more intense with the cards)

Copy link
Contributor

Choose a reason for hiding this comment

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

In my continued efforts to make this less jumpy, I decided to try to figure out a good value for this.

I landed on

skeletonCardCount={Math.min(displayData?.total ?? DEFAULT_PAGE_SIZE, DEFAULT_PAGE_SIZE)}

which doesn't account for pagination (specifically the last page being shorter than DEFAULT_PAGE_SIZE), but looks quite nice when combined with openedx/paragon#3992

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! That definitely looks better

@PKulkoRaccoonGang PKulkoRaccoonGang force-pushed the Peter_Kulko/catalog-data-table-search branch 2 times, most recently from 2dad3c0 to d44f7ff Compare November 18, 2025 13:00
@PKulkoRaccoonGang PKulkoRaccoonGang force-pushed the Peter_Kulko/catalog-data-table-search branch from d44f7ff to 4d25f17 Compare November 18, 2025 13:03
@github-project-automation github-project-automation bot moved this from In Eng Review to Done in Contributions Nov 18, 2025
@brian-smith-tcril
Copy link
Contributor

I did some local testing of the latest version of this branch and things are looking great!

I added

diff --git a/src/generic/course-card/index.tsx b/src/generic/course-card/index.tsx
index db3a2e7..ead8eab 100644
--- a/src/generic/course-card/index.tsx
+++ b/src/generic/course-card/index.tsx
@@ -31,6 +31,7 @@ export const CourseCard = ({ original: courseData, isLoading }: CourseCardProps)
         src={getFullImageUrl(courseData?.data.imageUrl)}
         fallbackSrc={noCourseImg}
         srcAlt={`${courseData?.data.content.displayName} ${courseData?.data.number}`}
+        skeletonDuringImageLoad={true}
       />
       <Card.Header
         title={courseData?.data.content.displayName}

to see how everything looks with the image skeleton loader, and ran into a couple small UX oddities.

I think they're on the Paragon side, so I made openedx/paragon#4002. I'd love to hear any thoughts on that issue.

@PKulkoRaccoonGang PKulkoRaccoonGang force-pushed the Peter_Kulko/catalog-data-table-search branch from b417dc8 to b714e69 Compare November 19, 2025 19:30
Copy link
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.

LGTM!

One last little comment but we can clean that up in a follow-up.

Thank you so much for working with me to get the UX to a great place with this PR. I'm extremely happy with how it turned out!

setSearchInput(value);
handleSearch(value);
}}
submitButtonLocation="external"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
submitButtonLocation="external"

With search-on-type I think we can move back to not having the external button.

@brian-smith-tcril brian-smith-tcril merged commit 5a4f439 into openedx:master Nov 20, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

FC Relates to an Axim Funded Contribution project open-source-contribution PR author is not from Axim or 2U

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants