Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move unsyncednoteids state from flux to redux #1871

Merged
merged 2 commits into from
Feb 3, 2020

Conversation

belcherj
Copy link
Contributor

@belcherj belcherj commented Jan 29, 2020

Fix

Moves unsyncednoteids state from flux to redux

Test

Review

  1. Have unsynced notes, you can probably create a few by deleting notes and then trashing permanently
  2. Toggle hamburger menu
  3. Hover over unsynced notes at bottom
  4. Does everything display the same as production

import {
setUnsyncedNoteIds,
toggleSimperiumConnectionStatus,
} from './state/ui/actions';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be able to import them all in one go if you find it easier

import * as actions from './state/actions';

dispatch( actions.ui.setUnsyncedNoteIds );

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like having them separated. If there were more than five or we had multiple /actions imports this would make sense to me.

@belcherj belcherj force-pushed the refactor/move-unsyncednoteids-to-redux branch from 55e7466 to 4ccf84c Compare February 3, 2020 20:24
@belcherj belcherj self-assigned this Feb 3, 2020
@belcherj belcherj marked this pull request as ready for review February 3, 2020 20:31
@belcherj belcherj requested a review from a team February 3, 2020 20:31
Copy link
Member

@dmsnell dmsnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a code-only review I haven't tested this. It's good to leave it like this for now, but hopefully in the future we'll rehaul how we detect this stuff. I'm pretty sure that node-simperium already gives us a way to trap unsync'd ids without deeply-coupling into it with our local-queue

@belcherj belcherj merged commit 4b915b1 into develop Feb 3, 2020
@belcherj belcherj deleted the refactor/move-unsyncednoteids-to-redux branch February 3, 2020 20:53
@dmsnell dmsnell mentioned this pull request Feb 19, 2020
41 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants