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

Add spotlight support for Simplenote #1140

Merged
merged 19 commits into from
Apr 17, 2024

Conversation

charliescheer
Copy link
Contributor

@charliescheer charliescheer commented Apr 11, 2024

Fix

It was requested in #1000 to index all of the notes in Simplenote so they can be searched for in spotlight. This is a great idea, so I have done just that!

With this PR you will be able to type in the text or title of any note into spotlight and the not will appear in the search results. Then you will be able to launch Simplenote directly to that note from spotlight.

fixes #1000

Test

Prep:
We want to confirm indexing and removing indexing before confirming the one time indexing of all of the existing notes. So at first we want to not run that indexing.... so:

  1. Go to SimplenoteAppDelegate L.126 and comment out the line [self indexSpotlightItemsIfNeeded]; so we don't index all your notes right away

Test adding and removing notes from the spotlight index:

  1. Open up Simplenote on another device that is logged into the same account and look for a note with some text. Open up spotlight and type that text in to the search field. Confirm that the note does not appear
  2. Open Simplenote on this device, open up that same note, add some text to it and then open a different note to force a save
  3. Return to Spotlight and search for some of the text in that note. Confirm that the note appears in spotlight as a search result
  4. select that search result. Confirm that Simplenote opens and goes to the note in question
  5. Now put that note in the trash. Go back to spotlight and search the same text, confirm that you do not see the note appear.

One time index:
6. now uncomment out the code we disabled before in the SimplenoteAppDelegate
7. relaunch the app. Give the app some time to do the indexing, this will happen in the background so you won't notice or get any indication that it is doing so.
8. Now search for text from any of your notes. Confirm it appears in spotlight
9. Put a break point in SimplenoteAppDelegate L602. Run the app again and confirm you do not hit this breakpoint. We only want to run that indexing once.

Review

(Required) Add instructions for reviewers. For example:

Only one developer is 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:

RELEASE-NOTES.txt was updated in ca5d1a with:

Added spotlight search support for notes

@charliescheer charliescheer added the [feature] search Anything related to searching. label Apr 11, 2024
@charliescheer charliescheer added this to the Future milestone Apr 11, 2024
@charliescheer charliescheer self-assigned this Apr 11, 2024
@roundhill
Copy link
Contributor

I think this feature is great but I'm wondering what you think of making it opt-in. I would imagine some users may not like the idea of their notes showing up in spotlight in case they have personal information in them. 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.

Code looks super sharp!!

+1 to what Dan said, perhaps we should have an opt-in setting, for privacy reasons?

Simplenote/SimplenoteAppDelegate+Swift.swift Outdated Show resolved Hide resolved
Simplenote/SimplenoteAppDelegate.m Outdated Show resolved Hide resolved
@dangermattic
Copy link
Collaborator

1 Warning
⚠️ View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.

Generated by 🚫 Danger

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.

Sending a couple questions (I'm not sure the indexSpotlightItemsIfNeeded API still makes sense?).

Other than that, REAL nice work!!!

:shipit:

Simplenote/CSSearchable+Helpers.swift Outdated Show resolved Hide resolved
Simplenote/CSSearchable+Helpers.swift Outdated Show resolved Hide resolved
Simplenote/SimplenoteAppDelegate+Swift.swift Outdated Show resolved Hide resolved
Simplenote/SimplenoteAppDelegate.m Outdated Show resolved Hide resolved
@charliescheer charliescheer merged commit 1ff5566 into trunk Apr 17, 2024
8 checks passed
@charliescheer charliescheer deleted the charlie/1000/add-spotlight-support branch April 17, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[feature] search Anything related to searching.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Spotlight Search Support
4 participants