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

Mobile: Fixes #11264: Fix editor shows nothing when there are no selected note IDs #11514

Merged

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Dec 14, 2024

Summary

This pull request fixes #11264 by preventing reducer.ts from clearing selectedNoteIds on mobile. Previously, some actions could result in the note the user was editing being removed from the selection. This caused the note editor to vanish.

To preserve the existing behavior on desktop, this pull request adds a property to the app state, allowSelectionInOtherFolders. When set to true, the reducer should avoid clearing selectedNoteIds when the selection is moved out of the current folder.

Notes

  • A previous attempt at a fix was merged into release-3.1. Due to an incorrectly resolved merge conflict, this fix was not applied to dev. This original fix worked by overriding the noteId prop passed to the note editor with the last non-null noteId. With this change, however, parts of the app that rely on state.selectedNoteIds to determine the note open in the editor remained broken when selectedNoteIds was cleared.
  • When the note viewer is closed, selectedNoteIds may be set to the ID of the last-opened note.

Testing

On web:

  1. Create notes A, B, C in different folders.
  2. Create links from C to B to A.
  3. Open the folder containing note C.
  4. Open note C, follow the link to note B, follow the link to note A.
  5. On a different Joplin client, move note B to a different folder.
  6. Sync the other Joplin client.
  7. Sync the original client (without leaving the note).
  8. Press the back button.
  9. Add a tag to note B.
    • The tag dialog should work.
  10. On the other Joplin client, move note B back to folder B.
  11. Sync both clients (without leaving note B).
  12. On the original client, verify that note B is still open.
  • Note: The folder picker may still display the original folder.
  1. Still on the original client, add a new tag to note B.
  • Verify that the tag dialog still works.
  1. Open the notes list, select multiple notes, and move them to a different folder.
  2. Verify that the notes have been moved.

On Android 13:

  1. Follow a link to a note in a different folder.
  2. Navigate to the device's app library.
  3. Switch back to Joplin.
  4. Verify that the same note is still open.
  5. Move the note to a different folder.
  6. Repeat steps 2-5.

@personalizedrefrigerator
Copy link
Collaborator Author

personalizedrefrigerator commented Dec 14, 2024

I'm closing this temporarily — although the note now renders after switching apps and returning, the tag dialog (which uses state.selectedNoteIds) is unable to add/remove tags.

A better fix may involve preventing selectedNoteIds from becoming empty.

@personalizedrefrigerator personalizedrefrigerator marked this pull request as draft December 15, 2024 21:53
@personalizedrefrigerator personalizedrefrigerator marked this pull request as ready for review December 16, 2024 17:34
@personalizedrefrigerator personalizedrefrigerator marked this pull request as draft December 16, 2024 17:44
@personalizedrefrigerator
Copy link
Collaborator Author

Possible issue: If:

  1. A user visits a note, then follows a link to a note in another folder ("note B").
  2. The user changes the parent folder of "note B" and follows a link from "note B" to "note C".
  3. The user presses the back button.

Then the selected folder ID may still not match the parent ID of the selected note. As a result, Notes.tsx may still clear selectedNoteIds (causing the original issue).

I'm converting this pull request to a draft until this is resolved.

@personalizedrefrigerator personalizedrefrigerator marked this pull request as ready for review December 16, 2024 22:45
@laurent22 laurent22 merged commit 5078341 into laurent22:dev Dec 18, 2024
7 checks passed
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.

Blank screen when leaving the app then coming back
2 participants