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

All: 1383 Clear selected Notes when switching Notebook #1387

Merged
merged 1 commit into from
Apr 4, 2019

Conversation

tkilaker
Copy link
Contributor

@tkilaker tkilaker commented Apr 3, 2019

This commit will fix #1383.

Tried several fixes, but I think this kind of quick-n-dirty fix is the most reasonable. Debugging shows that the root issue surrounds that the render method at NoteTest.jsx is run frequently, and a timing issue in the state can occur. Pausing on a break point actually "fixed" the issue.

@laurent22
Copy link
Owner

Thanks for the pull request, but if a note is empty in this array, we'll need to find the root cause of it. If it's unavoidable due to some race condition, then maybe we should exit the function, or remove the empty note from the array before processing it in the function.

@tkilaker
Copy link
Contributor Author

tkilaker commented Apr 3, 2019

Yes, it's a race condition. I did not see a trivial fix for it. I've amended the commit so that it will exit the function if any Note is null.

@laurent22
Copy link
Owner

I think the issue is that the selected notes are not reset in the FOLDER_SELECT action (in the reducer). If they are set to an empty array in there, it should fix this (and potentially other issues).

@tkilaker tkilaker changed the title Desktop: 1383 Add null check on Notes at context menu creation All: 1383 Add null check on Notes at context menu creation Apr 3, 2019
@tkilaker tkilaker changed the title All: 1383 Add null check on Notes at context menu creation All: 1383 Clear Selected Notes when switching Notebook Apr 3, 2019
@tkilaker tkilaker changed the title All: 1383 Clear Selected Notes when switching Notebook All: 1383 Clear selected Notes when switching Notebook Apr 3, 2019
@tkilaker
Copy link
Contributor Author

tkilaker commented Apr 3, 2019

I agree, that was what I was looking for. Thanks for the assistance!
And sorry for the temporary Git mess.

@laurent22
Copy link
Owner

Looks good, thanks for the fix!

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.

Multi selecting two Notes and then opening a different Notebook sets Joplin in a bad state
2 participants