-
Notifications
You must be signed in to change notification settings - Fork 76
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
[FC-0059] feat: Add filters/sorting for lib v2 tab #1117
[FC-0059] feat: Add filters/sorting for lib v2 tab #1117
Conversation
Thanks for the pull request, @yusuf-musleh! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
4a27b49
to
d5a56b8
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1117 +/- ##
==========================================
+ Coverage 92.67% 92.68% +0.01%
==========================================
Files 696 694 -2
Lines 12392 12359 -33
Branches 2714 2663 -51
==========================================
- Hits 11484 11455 -29
+ Misses 876 873 -3
+ Partials 32 31 -1 ☔ View full report in Codecov by Sentry. |
345b5fa
to
4f005bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 This looks and works great, thank you @yusuf-musleh !
- I tested this on my tutor dev stack using the PR test instructions.
- I read through the code
- I checked for accessibility issues by using the keyboard only to navigate the search/filter form.
-
Includes documentationN/A - User-facing strings are extracted for translation
0c2b719
to
49b7142
Compare
@openedx/2u-tnl this is ready for review! Thanks! |
@mphilbrick211 This is part of FC-0059, sorry we didn't have the FC number in the PR title. TNL is welcome to review but we can find a reviewer from one of our core contributors too :) |
Thanks, @bradenmacdonald! OK to have a CC review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @yusuf-musleh!
LGTM 👍 Great work!
I added a comment about a default value for the order
, but feel free to push back.
- I tested this on my tutor dev stack using the testing instructions
- I read through the code
-
I checked for accessibility issues N/A - Includes documentation
49b7142
to
6888565
Compare
src/studio-home/tabs-section/libraries-v2-tab/libraries-v2-filters/index.jsx
Outdated
Show resolved
Hide resolved
src/studio-home/tabs-section/libraries-v2-tab/libraries-v2-filters/index.jsx
Outdated
Show resolved
Hide resolved
src/studio-home/tabs-section/libraries-v2-tab/libraries-v2-filters/index.jsx
Outdated
Show resolved
Hide resolved
...o-home/tabs-section/libraries-v2-tab/libraries-v2-filters/libraries-v2-filter-menu/index.jsx
Outdated
Show resolved
Hide resolved
...o-home/tabs-section/libraries-v2-tab/libraries-v2-filters/libraries-v2-filter-menu/index.jsx
Outdated
Show resolved
Hide resolved
.../tabs-section/libraries-v2-tab/libraries-v2-filters/libraries-v2-order-filter-menu/index.jsx
Outdated
Show resolved
Hide resolved
.../tabs-section/libraries-v2-tab/libraries-v2-filters/libraries-v2-order-filter-menu/index.jsx
Outdated
Show resolved
Hide resolved
6888565
to
2469f04
Compare
fd13676
to
5d7591a
Compare
34d8ed6
to
07542c0
Compare
@yusuf-musleh 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Description
This PR adds searching and sorting functionality in the V2 Libraries tab.
Supporting information
Related Tickets:
order
query param to lib v2 API edx-platform#35005Testing instructions
order
query param to lib v2 API edx-platform#35005 running locally as wellPrivate-ref: FAL-3752