-
Notifications
You must be signed in to change notification settings - Fork 285
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
Add Append note Intent #1582
Add Append note Intent #1582
Conversation
You can test the changes in simplenote-ios from this Pull Request by:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving a note, already discussed over Slack!.
I've found a glitch while testing this PR:
- Add a new
Append note
Shortcut - Invoke the shortcut multiple times
- Launch Simplenote
As a result, only the last text you've appended will be visible in the note.
return AppendNoteIntentResponse(code: .failure, userActivity: nil) | ||
} | ||
|
||
note.content? += "\n\n\(content)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may wanna do something like:
let oldContent = note.content ?? ""
note.content = oldContent + "\n\n\(content)"
(Or so?) Otherwise whenever the content is currently nil, the new text would get lost
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Verified you can, as of now, append text to an Entry
✅ Verified that multiple Append OPs don't overwrite each other
Nice work Charlie!!
@@ -61,10 +61,46 @@ extension Uploader: URLSessionTaskDelegate { | |||
} | |||
} | |||
|
|||
class Downloader: NSObject { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maaaybe we should either extract this into its own file, OR just rename as SimperiumREST.swift
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved it to it's own file
return AppendNoteIntentResponse(code: .failure, userActivity: nil) | ||
} | ||
|
||
note.content = existingContent + "\n\n\(content)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we'd rather using + \n
(rather than a double newline. Dunno?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Fix
So continuing on with the shortcut improvements. One of the requested shortcuts is to be able to append to an existing note. Makes sense that you would want to be able to use shortcuts to easily add to one of your existing notes. So in this PR I have done just that.
This PR adds a new intent Append Note. With Append note you can specify an existing note and content that you want to add to it. You can also choose to select the note and the content at the time which makes this shortcut a pretty versatile way to add content to existing Simplenote notes from outside of the app.
Test
A few things to consider while testing this:
Review
(Required) Add instructions for reviewers. For example:
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:(When the last of the shortcuts are added then I will update the release notes)