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

Tests: cursorNext/cursorPrev #831

Merged
merged 5 commits into from
Aug 18, 2020
Merged

Conversation

aqkhan
Copy link
Contributor

@aqkhan aqkhan commented Aug 17, 2020

Fixes #806

@aqkhan
Copy link
Contributor Author

aqkhan commented Aug 17, 2020

@raineorshine I've found the following issues with the action-creators cursorNext and cursorPrev:

  • Both the action-creators do not work with the sorted context as expected.
  • Both the action-creators do not move the cursor to first child of root when the cursor is not set.

I've fixed the second issue, i.e move the cursor to first child of root when the it is not set but need more time to fix the first issue. I will move forward after your approval.

@raineorshine
Copy link
Contributor

Nicely done!

I did some minor refactors, but otherwise looks great:

  • Moved tests to /action-creators/__tests__/ directory.
  • Removed unnecessary async/await (it's not obvious that this works since importText appears to be asynchronous and needs to be updated).
  • Dispatch multiple actions at once using redux-multi.
  • Remove rank from toMatchObject since that creates an unnecessary dependency on importText's somewhat arbitrary ranks.

@raineorshine
Copy link
Contributor

I will merge and you can open another PR for the sort issue! Thanks!

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.

Tests: cursorNext/cursorPrev
2 participants