Fix history logic #504

Closed

btzr-io wants to merge 5 commits from patch-2 into master

btzr-io commented

2017-08-27 04:13:13 +02:00

(Migrated from github.com)

Todo

~~Fix / debug https://github.com/lbryio/lbry-app/issues/499~~

Test this.
Try the hard way: https://github.com/lbryio/lbry-app/issues/499#issuecomment-325423807
Fix / debug https://gihub.com/lbryio/lbry-app/issues/499#issuecomment-325147682
Fix duplicated history bug.
Update changelog.md.

### Todo ~Fix / debug https://github.com/lbryio/lbry-app/issues/499~ - [ ] Test this. - [ ] Try the hard way: https://github.com/lbryio/lbry-app/issues/499#issuecomment-325423807 - [x] Fix / debug https://gihub.com/lbryio/lbry-app/issues/499#issuecomment-325147682 - [x] Fix duplicated history bug. - [x] Update changelog.md.

kauffj (Migrated from github.com) reviewed 2017-08-27 04:13:13 +02:00

btzr-io commented

2017-08-27 22:57:29 +02:00

(Migrated from github.com)

please test this 😛 ^^

please test this :stuck_out_tongue: ^^

btzr-io commented

2017-08-29 05:41:25 +02:00

(Migrated from github.com)

Fix conflicts 49ab36d ^^

Fix conflicts 49ab36d ^^

kauffj commented

2017-08-30 19:45:15 +02:00

(Migrated from github.com)

As I suspected, this is possible to do with just one stack. I found some other simplifications as well. You can see the full changeset here:

f6976c3feb

The total line changes make it look worse than it was, as I decided we ought to split navigation into it's own set of actions and reducers. If you look at just the reducer for HISTORY_NAVIGATE, you'll see it's really just a simplification.

Most importantly, I noticed a reducer that was directly modifying the state of an object. In React/Redux reducers, object and array state ought not to be directly mutated, but instead the objects should be copied and updated. See: https://github.com/reactjs/redux/blob/master/docs/recipes/reducers/ImmutableUpdatePatterns.md

As I suspected, this is possible to do with just one stack. I found some other simplifications as well. You can see the full changeset here: https://github.com/lbryio/lbry-app/commit/f6976c3feb5a049e5071d027f520a11dbba3b4f0 The total line changes make it look worse than it was, as I decided we ought to split navigation into it's own set of actions and reducers. If you look at just the reducer for `HISTORY_NAVIGATE`, you'll see it's really just a simplification. Most importantly, I noticed a reducer that was directly modifying the state of an object. In React/Redux reducers, object and array state ought not to be directly mutated, but instead the objects should be copied and updated. See: https://github.com/reactjs/redux/blob/master/docs/recipes/reducers/ImmutableUpdatePatterns.md

👍 1 🎉 1

btzr-io commented

2017-08-31 06:41:49 +02:00

(Migrated from github.com)

@kauffj Ok, sorry I didn't have time to finish this, thanks for fixing my mess 🙃 ^^
BTW your implementation looks awesome 👍

@kauffj Ok, sorry I didn't have time to finish this, thanks for fixing my mess :upside_down_face: ^^ BTW your implementation looks awesome :+1: