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

Delete note if the content is completely removed #1665

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

charliescheer
Copy link
Contributor

@charliescheer charliescheer commented Oct 19, 2024

Fix

It was brought up in an issue for SNMac, that if a note is empty we leave it in the note list and it would be nice if they deleted themselves. I thought about this for a bit and decided that I agreed, but I wanted to make sure that we only moved the notes to the trash, that way the history for the note still exists and a user could reclaim the lost content if they wanted. So in this PR I have done that.

iOS already deleted new notes that were empty, so this PR simply updates that logic to not exclude existing notes. If a user deletes all the content in a note and then exits the editor the note will be moved to the trash. This is the same behavior as in apple notes.

Test

Confirm new notes with content are not deleted

  1. Create a new note and add some content to it
  2. Close the editor
  • Confirm your new note continues to exist
  • Relaunch the app and confirm your new note was saved to the DB

Confirm new notes without content are deleted

  1. Create a new note, do not add content to it
  2. Close the editor
  • Confirm that your new note was deleted

Confirm an existing note that has all it's content removed, will be deleted

  1. Go to a note you don't mind losing.
  2. Delete all of the text content in the note
  3. Exit the editor.
  • Confirm that your note is deleted from the notes list
  1. Go to the note trash, find the top note labeled "New Note..." restore it
  2. Go to the regular notes list, open the note you restored, go to the notes history
  • Confirm you can restore that notes history

Review

(Required) Add instructions for reviewers. For example:

Only one developer and one designer are required to review these changes, but anyone can perform the review.

Release

(Required) Add a concise statement to RELEASE-NOTES.txt if the changes should be included in release notes. Include details about updating the notes in this section. For example:

These changes do not require release notes.

@charliescheer charliescheer added [feature] note list Anything related to the note list. [feature] editor Anything related to the editor. labels Oct 19, 2024
@charliescheer charliescheer added this to the 4.56 milestone Oct 19, 2024
@charliescheer charliescheer self-assigned this Oct 19, 2024
@dangermattic
Copy link
Collaborator

1 Warning
⚠️ This PR is assigned to the milestone 4.56. This milestone is due in less than 4 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

Simplenote Prototype Build📲 You can test the changes from this Pull Request in Simplenote Prototype Build by scanning the QR code below to install the corresponding build.
App NameSimplenote Prototype Build Simplenote Prototype Build
Build Numberpr1665-c71dc66-0192a2aa-7300-4562-958e-e55644f721b7
Version4.54
Bundle IDcom.codality.NotationalFlow.Alpha
Commitc71dc66
App Center BuildSimplenote - Installable Builds #387
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@jleandroperez
Copy link
Contributor

@charliescheer I'm not 100% sure this change is what we want: you'd loose the ability to use the Revision History slider to recover the content.

The note would end up in Trash, so you'd need to manually recover, before you're able to use the version slider.

WDYT?

Copy link
Contributor

@jleandroperez jleandroperez left a comment

Choose a reason for hiding this comment

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

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[feature] editor Anything related to the editor. [feature] note list Anything related to the note list.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants