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 NSUserActivity to rooms to appear in spotlight search #4865

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

kloenk
Copy link
Contributor

@kloenk kloenk commented Sep 18, 2021

This also allows shortcuts to open the app directly opening a room.

Pull Request Checklist

  • I read the contributing guide
  • UI change has been tested on both light and dark themes, in portrait and landscape orientations and on iPhone and iPad simulators
  • Pull request is based on the develop branch
  • Pull request contains a changelog file in ./changelog.d
  • Pull request includes screenshots or videos of UI changes
  • Pull request includes a sign off

Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution Finn. I've added a few inline comments.

Would you be able to add a changelog file in ./changelog.d and a sign off for us too please?

Riot/Modules/Room/RoomViewController.m Outdated Show resolved Hide resolved
Riot/Modules/Room/RoomViewController.m Outdated Show resolved Hide resolved
Riot/Modules/Application/LegacyAppDelegate.m Outdated Show resolved Hide resolved
@kloenk kloenk force-pushed the userActivity branch 3 times, most recently from 9328f2b to 9219fa1 Compare September 22, 2021 18:25
@kloenk kloenk changed the base branch from develop to Kloenk/userActivity September 23, 2021 08:52
@kloenk kloenk force-pushed the userActivity branch 3 times, most recently from 8d0a7e3 to f5e88e8 Compare September 27, 2021 09:28
@kloenk kloenk changed the base branch from Kloenk/userActivity to develop September 27, 2021 09:28
@kloenk kloenk force-pushed the userActivity branch 2 times, most recently from 9a92b69 to 1b9c15d Compare September 27, 2021 11:13
@kloenk kloenk force-pushed the userActivity branch 5 times, most recently from e10102c to 8bf64aa Compare September 28, 2021 08:16
@pixlwave
Copy link
Member

For this initial implementation, would you mind removing the CSSearchableItemAttributeSet and CSSearchableItem parts and relying on the userActivity.isEligibleForSearch = true for spotlight support? I believe the only visible change this would have is the lack of a subtitle - which may be a positive thing as it could be confusing in the Shortcuts app.

It would be interesting to have spotlight functionality in its own service eventually, so that updates to the index can happen automatically. Then the UserActivityService could request the appropriate attributes to attach to the activity from that service.

@kloenk
Copy link
Contributor Author

kloenk commented Oct 31, 2021

Let's anchor 3086234 here, so it's easier to find how to add it back later on

kloenk and others added 5 commits October 31, 2021 19:13
Signed-off-by: Finn Behrens <me@kloenk.dev>
Signed-off-by: Finn Behrens <me@kloenk.dev>
Signed-off-by: Finn Behrens <me@kloenk.dev>
Signed-off-by: Finn Behrens <me@kloenk.dev>
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

3 participants