Preferences #56

2019-10-11T18:23:35+02:00

akinwale commented

2019-10-11 18:23:35 +02:00

(Migrated from github.com)

No description provided.

neb-b (Migrated from github.com) reviewed 2019-10-11 18:23:35 +02:00

kauffj (Migrated from github.com) reviewed 2019-10-11 22:51:25 +02:00

kauffj (Migrated from github.com) left a comment

I think it would make sense to consider splitting all sync behavior in to a separate component.

This component could also receive the list of channels, the tags they follow, and any other relevant variables and push the state any time they change. This would keep all sync behavior cleanly separated in a single file as well as make it easier to prevent multiple syncs from firing concurrently (which is unlikely to impossible in the current design, but could conceivably be possible as more events/behavior trigger a sync).

I think it would make sense to consider splitting all sync behavior in to a separate component. This component could also receive the list of channels, the tags they follow, and any other relevant variables and push the state any time they change. This would keep all sync behavior cleanly separated in a single file as well as make it easier to prevent multiple syncs from firing concurrently (which is unlikely to impossible in the current design, but could conceivably be possible as more events/behavior trigger a sync).

src/component/AppNavigator.js

					
				@ -29,7 +29,7 @@ import {

				import { connect } from 'react-redux';

kauffj (Migrated from github.com) commented

2019-10-11 22:47:33 +02:00

none of the logic in this file appears to affect the presentation or rendering of this component

is there a design that would separate this out more cleanly? maybe just make it it's own component?

none of the logic in this file appears to affect the presentation or rendering of this component is there a design that would separate this out more cleanly? maybe just make it it's own component?

src/component/AppNavigator.js

					
				@ -323,3 +336,3 @@

				    const { dispatch, user } = this.props;

				    const { dispatch, user, hashChanged } = this.props;

				    if (this.state.verifyPending && this.emailVerifyCheckInterval > 0 && user && user.has_verified_email) {

				      clearInterval(this.emailVerifyCheckInterval);

kauffj (Migrated from github.com) commented

2019-10-11 22:48:15 +02:00

you should probably store the result of this setInterval call and call clearInterval on unmount

you should probably store the result of this `setInterval` call and call `clearInterval` on unmount

src/component/AppNavigator.js

					
				@ -324,3 +336,4 @@

				    const { dispatch, user, hashChanged } = this.props;

				    if (this.state.verifyPending && this.emailVerifyCheckInterval > 0 && user && user.has_verified_email) {

				      clearInterval(this.emailVerifyCheckInterval);

				      AsyncStorage.setItem(Constants.KEY_EMAIL_VERIFY_PENDING, 'false');