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

open card browser and search using deep links #11899

Merged
merged 3 commits into from
Aug 20, 2022

Conversation

krmanik
Copy link
Member

@krmanik krmanik commented Jul 20, 2022

Pull Request template

Purpose / Description

Open card browser from another app with search query

Fixes

Fixes #11885

Approach

  1. Create intent-filter in Cardbrowser activity with scheme="anki", host="x-callback-url" and path="/cardbrowser"
  2. Get intent data using getQueryParameter

How Has This Been Tested?

Tested using chrome and https://krmanik.github.io/deeplinks-intent-test/

  1. Load the page on chrome browser on android device
  2. Input the fields to make intent like this for searching test
intent://x-callback-url/cardbrowser?search=test#Intent;scheme=anki;package=com.ichi2.anki;end
  1. The webpage creates a tag and href value with above intent
    a) Searching nid

    <a href="intent://x-callback-url/cardbrowser?search=nid:1658178059263#Intent;scheme=anki;package=com.ichi2.anki;end">Search nid </a>
    

    b) Searching some text

    <a href="intent://x-callback-url/cardbrowser?search=some text#Intent;scheme=anki;package=com.ichi2.anki;end">Search text</a>
    
  2. Clicking on Open action will open the card browser with search

  3. Also following can be used to test
    a) Using adb

    adb shell am start 
         -W -a android.intent.action.VIEW 
         -d "anki://x-callback-url/cardbrowser?search=nid:1658178059263" com.ichi2.anki
    

    b) Using window.location

    ankiUrlScheme = "anki://x-callback-url/cardbrowser?search=nid:1658178059263"
    document.location = ankiUrlScheme;
    

Learning (optional, can help others)

https://developer.android.com/training/app-links/deep-linking
https://developer.chrome.com/docs/multidevice/android/intents/

Links to blog posts, patterns, libraries or addons used to solve this problem

Checklist

Please, go through these checks before submitting the PR.

  • You have not changed whitespace unnecessarily (it makes diffs hard to read)
  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • Your code follows the style of the project (e.g. never omit braces in if statements)
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@mikehardy
Copy link
Member

mikehardy commented Jul 20, 2022

Cool! Just a question, what happens if the link has the x-callback-url stuff / looks exactly the same as the iOS version from ankimobile docs? Just curious since true cross platform interop is such a superpower, it is worth some code to manage it if possible

@krmanik
Copy link
Member Author

krmanik commented Jul 20, 2022

The x-callback-url only supported on iOS devices. I have updated it to match the url scheme and it is working similar to AnkiMobile.

In safari or chrome for iOS devices, following can be used. Also addnote and infoForAdding supported on AnkiMobile currently.

ankiUrlScheme = "anki://x-callback-url/addnote?profile=User%201&type=Basic&deck=Default&fldFront=front%20text&fldBack=back%20text"
document.location = ankiUrlScheme;

For AnkiDroid, cardbrowser

ankiUrlScheme = "anki://x-callback-url/cardbrowser?search_query=nid:1658178059263"
document.location = ankiUrlScheme;

Also in docs we have to mention that x-callback-url used to match the AnkiMobile URL scheme

Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

That looks surprisingly easy / "works as documented", and though the feature (search here, vs add note etc on ios) varies from iOS it has compatible construction. This looks like a style with lots of future potential to me.

@mikehardy
Copy link
Member

Hey @dae if you have a minute, this is an ecosystem cross-compatibility thing. My hope is that all URLs will work on all platforms and I'd love it if you could confirm we're meeting that goal with our support here

@mikehardy mikehardy added the Needs Second Approval Has one approval, one more approval to merge label Aug 19, 2022
@dae
Copy link
Contributor

dae commented Aug 19, 2022

ankiUrlScheme = "anki://x-callback-url/cardbrowser?search_query=nid:1658178059263"
document.location = ankiUrlScheme;

That looks like the syntax AnkiMobile uses, though please note it only supports adding notes at the moment, and does not support searching.

Minor bikeshedding on the URL - how about this instead?

anki://x-callback-url/browser?search=nid:1658178059263

@mikehardy
Copy link
Member

it only supports adding notes at the moment, and does not support searching

But now that you're aware adding this we can stay in sync when/if you add searching :-). Good to hear we're on-target and I agree "search" is unambiguous enough while being shorter and reading better for non-developers

@mikehardy mikehardy added the Needs Author Reply Waiting for a reply from the original author label Aug 19, 2022
@BrayanDSO
Copy link
Member

anki://x-callback-url/browser?search=nid:1658178059263

I like the cardbrowserbrowser change too

@krmanik krmanik removed the Needs Author Reply Waiting for a reply from the original author label Aug 20, 2022
Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

Looks great!
All of Damien's URL suggestions ingested now

Copy link
Member

@BrayanDSO BrayanDSO left a comment

Choose a reason for hiding this comment

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

LGTM too. Awesome feature

@BrayanDSO BrayanDSO added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Second Approval Has one approval, one more approval to merge labels Aug 20, 2022
@mikehardy
Copy link
Member

The emulator tests are erroring with different errors but every time I look at them it's the system going out of memory, I'll see if I can tune that quickly, the false-negative rate is atrocious. Thanks for the restart

@mikehardy mikehardy merged commit 5c72117 into ankidroid:main Aug 20, 2022
@github-actions github-actions bot added this to the 2.16 release milestone Aug 20, 2022
@github-actions github-actions bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Aug 20, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2022

Hi there @krmanik! This is the OpenCollective Notice for PRs merged from 2022-08-01 through 2022-08-31

If you are interested in compensation for this work, the process with details is here:

https://github.com/ankidroid/Anki-Android/wiki/OpenCollective-Payment-Process#how-to-get-paid

We only post one comment per person per month to avoid spamming you, regardless of the number of PRs merged, but this note applies to all PRs merged for this month

Please note that GSoC contributions are okay for this process. Our philosophy is that our users have donated to AnkiDroid for all contributions. The only PRs that will not go through the OpenCollective process are ones directly related to am accepted GSoC project from a selected participant, since those receive a stipend from GSoC itself.

Please understand that our monthly budget is never guaranteed to cover all claims - the cap on payments-per-person may be lower, but we try to make our process as fair and transparent as possible, we just need your understanding.

Thanks!

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.

Intent/Activity API for opening the Card Browser
4 participants