[BB-9101] Add missing default filter #65
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## asu-moe/redwood-css #65 +/- ##
=======================================================
- Coverage 92.54% 92.48% -0.06%
=======================================================
Files 706 706
Lines 12515 12529 +14
Branches 2728 2737 +9
=======================================================
+ Hits 11582 11588 +6
- Misses 898 906 +8
Partials 35 35 ☔ View full report in Codecov by Sentry. |
pkulkark
force-pushed
the
pooja/bb9101-missing-fix
branch
from
051a2d7 to
9ceb989
Compare
pkulkark
force-pushed
the
pooja/bb9101-missing-fix
branch
from
3bb1e37 to
fa800c9
Compare
| if (menuType !== 'courseTypes') { | ||
| return; | ||
| } |
There was a problem hiding this comment.
@pkulkark Why do we only make this behavior work for one type of the menu? Shouldn't default value work correctly for all menu types?
There was a problem hiding this comment.
@Cup0fCoffee It does, but since the same filter component is used for two different menus rendered one after another, the second one resets the first. And since we don't need to change the default behaviour of the second filter, I decided this option would be better. What do you think?
| @@ -26,6 +27,13 @@ const CoursesFilterMenu = ({ | |||
| if (cleanFilters) { | |||
| setItemMenuSelected(defaultItemSelectedText); | |||
There was a problem hiding this comment.
@pkulkark Don't you just need to call the onItemMenuSelected here? Or even better:
| setItemMenuSelected(defaultItemSelectedText); | |
| const defaultItemSelectedValue = menuItems.find(item => item.name === defaultItemSelectedText); | |
| handleCourseTypeSelected(defaultItemSelectedText, defaultItemSelectedValue); |
There was a problem hiding this comment.
@Cup0fCoffee I actually couldn't fully figure out how the cleanFilters option could be updated, so I didn't change the existing behaviour.
|
Closing this in favor of #66 |
This PR adds the missing line from #63 which sets the default filter to active courses.