Addressing review on D6969 - creating a separate reducer for picked communities
Details
Details
Run web app, checked that pressing a drawer item in the Calendar tab still changes the state properly
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| web/redux/community-picker-reducer.js | ||
|---|---|---|
| 4–6 ↗ | (On Diff #24779) | These imports can be merged |
| 20–25 ↗ | (On Diff #24779) | Usually a reducer acts only on a single store. I guess there might be some cases where multiple stores might be reasonable, but probably not this time. The reason is that we already have reduceCalendarFilters and all the changes to calendarFilters should be made by it. So we should add handling for these actions there and make reduceCommunityPickerStore act on just one store. |
Comment Actions
Merge imports
| web/redux/community-picker-reducer.js | ||
|---|---|---|
| 20–25 ↗ | (On Diff #24779) | reduceCalendarFilters is in lib/, so it cannot handle these actions, which are only on web. There is no other place on web where calendarFilters are changed (reduced). |
Comment Actions
I agree we should only be changing calendar filters in one place. Can we move the logic for changing them here to lib?
Comment Actions
If we move actions to lib, we can simplify the reducer and avoid splitting the login in two places.
| web/redux/community-picker-reducer.js | ||
|---|---|---|
| 13 ↗ | (On Diff #26744) | Why do we need threadStore? |