search

cheshire137 / gh-notifications-snoozer

Lists and filters and snoozing pull requests, oh my! This is an app for managing your notifications on GitHub by way of filtering issues and pull requests that are of interest to you.
MIT License
15 stars 10 forks source link

Move filter state into Redux #108

Closed probablycorey closed 7 years ago

probablycorey commented 7 years ago

This PR got a too big, but I couldn't figure out a good way to incrementally move the state from ElectronConfig into Redux. The TL;DR of this PR is:

  1. Removed Filter, Filters and LastFilter because the state is now stored via https://github.com/cheshire137/gh-notifications-snoozer/compare/move-filter-to-redux?expand=1#diff-f27775794e657dcf597e48b0c1c74b62
  2. Removed some callbacks that aren't needed anymore
  3. Made sure your filters are copied over to the new state (via the DefaultFilters class)

The behavior of the app should be unchanged.

cc @cheshire137

cheshire137 commented 7 years ago

I resolved the conflicts. However, testing this one out, the same tasks are shown in the tasks list even after I change filters. I would expect to only see tasks for the selected filter.

probablycorey commented 7 years ago

Thanks for fixing those merge conflicts!

I tracked down the bug for filters not being applied. It is at https://github.com/cheshire137/gh-notifications-snoozer/blob/move-filter-to-redux/src/components/TaskList/TaskList.jsx#L88.

At that point, this.props.activeFilter hasn't been updated to be the filter dispatched in line 87. I assumed it would be. I still need to look deeper into how react-redux works to figure out how to fix this.