Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: adds sort widget to search manager and library component page [FC-0059] #51

Conversation

pomegranited
Copy link
Member

@pomegranited pomegranited commented Jul 10, 2024

Please use this PR for code review.
See openedx#1147 for testing instructions.

Private-ref: FAL-3758

yusuf-musleh and others added 30 commits June 3, 2024 15:44
When lib mode is set to "mixed", both "Libraries" and "Legacy Libraries" tabs are show in the Studio Home. When "Libraries" is clicked, v2 libraries are fetched, when "Legacy Libraries" is clicked, v1 libraries are fetched.

When lib mode is set to "v1 only" or "v2 only", only one tab "Libraries" is show and only the respective libraries are fetched when the tab is clicked.
This is to switch between different library modes.
The path updates when selecting tabs, when accessing the url with the path directly it will open its respective tab. Navigating using the browser back/forward buttons is also supported.
This commit is temporary as the current frontend build system in tests doesnt support TS syntax. That should be fixed soon, and this commit should be removed.
This is a temporary commit since there are currently no webpack loaders that support tsx files in the test running. This commit should be removed once that is fixed upstream.
When lib mode is set to "mixed", both "Libraries" and "Legacy Libraries" tabs are show in the Studio Home. When "Libraries" is clicked, v2 libraries are fetched, when "Legacy Libraries" is clicked, v1 libraries are fetched.

When lib mode is set to "v1 only" or "v2 only", only one tab "Libraries" is show and only the respective libraries are fetched when the tab is clicked.
This is to switch between different library modes.
The path updates when selecting tabs, when accessing the url with the path directly it will open its respective tab. Navigating using the browser back/forward buttons is also supported.
This commit is temporary as the current frontend build system in tests doesnt support TS syntax. That should be fixed soon, and this commit should be removed.
This is a temporary commit since there are currently no webpack loaders that support tsx files in the test running. This commit should be removed once that is fixed upstream.
Copy link

codecov bot commented Jul 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.68%. Comparing base (3400f71) to head (61af79d).
Report is 17 commits behind head on rpenido/fal-3764-library-home-filter-by-content-type.

Additional details and impacted files
@@                                   Coverage Diff                                    @@
##           rpenido/fal-3764-library-home-filter-by-content-type      #51      +/-   ##
========================================================================================
+ Coverage                                                 92.65%   92.68%   +0.02%     
========================================================================================
  Files                                                       696      693       -3     
  Lines                                                     12365    12339      -26     
  Branches                                                   2707     2694      -13     
========================================================================================
- Hits                                                      11457    11436      -21     
+ Misses                                                      876      872       -4     
+ Partials                                                     32       31       -1     

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

@pomegranited
Copy link
Member Author

pomegranited commented Jul 10, 2024

@yusuf-musleh I will address this point on the ticket today by modifying the SearchContext:

The sort option should be reflected in the URL, e.g. ?sort=created but should not create a new history entry, so pressing BACK in the browser goes back to the previous page, not the previous sort option.

... and I'll add tests :)

@yusuf-musleh
Copy link

@pomegranited This is looking great so far! I've tested it out and its working quite well. I like the use of an enum for the sort options, very clean. And this PR would definitely make things easier for me working on the "Recently Modified" section.

Copy link
Member

@rpenido rpenido left a comment

Choose a reason for hiding this comment

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

LGTM 👍
Thank you for your great work here @pomegranited. I liked the useStateWithUrlSearchParam hook. Very elegant!

src/search-manager/SearchManager.ts Outdated Show resolved Hide resolved
* search parameter when returning/setting the state variable.
*
*/
function useStateWithUrlSearchParam<Type>(
Copy link
Member

Choose a reason for hiding this comment

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

Liked this! Good work @pomegranited !

pomegranited and others added 3 commits July 20, 2024 08:22
* feat: Add Recently Modified library section
* feat: Add "View All" to library sections
  The "View All" action appears on sections that pass in a view all action and contain content that exceeds the defined preview limit, which defaults to 4.
* feat: Use intl library section titles
* test: Update tests
There's still a difference with the arrow icon used -- DropDown uses a
css-generated hollow caret, but the filters use the svg solid caret Icon.
@rpenido rpenido force-pushed the rpenido/fal-3764-library-home-filter-by-content-type branch 2 times, most recently from 3d66e47 to 317c3e7 Compare July 23, 2024 17:42
@pomegranited
Copy link
Member Author

Closed in favour of openedx#1147

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants