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

iOS deep link refresh #1225

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open

iOS deep link refresh #1225

wants to merge 27 commits into from

Conversation

DaveMeadAdjust
Copy link
Collaborator

Related issues

N/A

Changes

Refreshes the entire iOS deep linking implementation to be more comprehensive, accurate, and up-to-date.

Required translations

Platform ZH JA KO
iOS

Copy link

vercel bot commented Dec 1, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dev-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 2, 2024 9:28am

@DaveMeadAdjust DaveMeadAdjust changed the title Ios deep link refresh iOS deep link refresh Dec 1, 2024

This scheme will work for your production **and** debug builds.

### Link Resolution domains {#link-resolution-domains}
Copy link
Member

Choose a reason for hiding this comment

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

I could not find the mentioned of link resolution domains in the rewritten page, just wanted to make sure that this was intentional or something missed. I'm aware that we already have a full page dedicated to it here

Copy link
Collaborator Author

@DaveMeadAdjust DaveMeadAdjust Dec 5, 2024

Choose a reason for hiding this comment

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

Yes, this was intentional. Reasons:

  • Link Resolution is a feature that only a relatively small subset of customers use. Removing it from the main setup page simplifies the setup for most customers.
  • Things have changed since I wrote the previous iteration of the documentation. We have added short link resolution, which has its own method called processAndResolveDeeplink. We've had multiple customers that were confused because they were mixing these two methods up.
  • Link Resolution requires the customer to set up non-Adjust infrastructure and/or configuration with third parties (usually ESPs). This part is usually more cumbersome for most customers. Since Link Resolution has these additional requirements, it makes sense to me to move all Link Resolution details to the dedicated page. For the SDK part on the Link Resolution page, I can simply reference the main documentation and show an updated version of the DeeplinkHandler example class, which will show the Link Resolution -> processAndResolveDeeplink -> app's deep link handling sequence that customers have to set up.
  • The Link Resolution steps were included in the main steps before, which complicated sections like Associated Domains setup. It's better to just show the go.link Associated Domains setup in the main documentation, and then in the Link Resolution section, just point to the main configuration page and say "also add your Link Resolution domain using these steps." I can add this code to the Link Resolution page before this pull request gets merged.

6. (Optional) Turn on **Send data to App Store Connect** and enter the **Apple provider ID** to send data to App Store Connect App Analytics.
7. Select **Save**.

## Set up deep links with a custom URL scheme {#set-up-deep-links-with-a-custom-url-scheme}
Copy link
Member

Choose a reason for hiding this comment

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

I could not find these instructions in the new files. Are they no longer needed or are present in some other page (perhaps in the dashboard part)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I link to this page:

https://help.adjust.com/en/article/platforms-ios-android-amazon-microsoft

They have changed the dashboard logic before (you have to set up android app links before you can create a deep link, etc.). So I would say it's better to keep it in the same place. I'll update to using this link though, as it's more direct for iOS.

https://help.adjust.com/en/article/platforms-ios-android-amazon-microsoft#ios-apple-app-store-app

5. Your app displays its preliminary content, such as onboarding screens and user login, if applicable.
6. Your app retrieves the deep link from the Adjust SDK and handles the deep link.

## Setup {#setup}
Copy link
Member

Choose a reason for hiding this comment

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

I can see that the example how to setup deferred deeplinking is in this new page but the context (what it is and how it works) is gone. Is this something clients are aware enough (or don't need to know) or was it transfered to some other part?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is a good point. There is a similar section here:

Most apps have some kind of onboarding process (for example: ATT prompt, onboarding screens, login prompt). When setting up deferred deep linking, you have to ensure that onboarding and launching deferred deep links don't interfere with each other. One approach is to wait until after onboarding to launch deferred deep links. Here's how it works in the code examples:

This has the deferred deep linking sequence. I can add more context on deferred deep linking to this section.

</Tab>
</Tabs>

## Set up Adjust LinkMe {#set-up-adjust-linkme}
Copy link
Member

Choose a reason for hiding this comment

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

I see that the campaign set up page is still up and linking to this section. Will/Is this technical detail be somewhere else or is it something we don't want to higlight?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a good point. LinkMe is a not a commonly used feature because it requires the customer to introduce a iOS pasteboard dialog into their app for marginal benefit (not many iOS devices require LinkMe for deferred deep linking), so I definitely wouldn't put it in the main docs. I think maybe we should have an "additional deep linking features" page where we put a bunch of smaller miscellaneous features like this. I can add this page and see if we can update the help center link to point to it.

@Aditi3
Copy link
Member

Aditi3 commented Dec 5, 2024

Hi @DaveMeadAdjust My feedback on code snippets is that we should not suggest optimizing our codebase and Adjust's APIs in developer documentation. Optimization and usage should be determined by users or customers based on their application requirements.

https://dev-docs-git-ios-deep-link-refresh-product-content.vercel.app/en/sdk/ios/features/deep-links/set-up-deep-linking

By demonstrating a wrapper class, the Deep linking handler will set one particular way of handling. Another point to consider is that the usage of Dispatch.main.async should not be demonstrated either.

Screenshot 2024-12-05 at 12 46 12 PM Screenshot 2024-12-05 at 12 48 12 PM

I recommend continuing to demonstrate the code snippet using Apple's method. Like here. At first glance, I also found the DeeplinkHandler instances quite confusing.

let incomingLink = userActivity.webpageURL
{
// Handle incoming universal link
DeeplinkHandler.handleDeeplink(incomingLink)
Copy link
Member

Choose a reason for hiding this comment

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

What is DeeplinkHandler?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

By demonstrating a wrapper class, the Deep linking handler will set one particular way of handling.

I'm aiming to accomplish a handful of goals with the DeeplinkHandler example class:

  • Show the Adjust SDK interaction.
  • Show where the customer's deep linking handling goes (this has a dependency on the output of the Adjust SDK).
  • Centralize the code for the above two points in one place. Otherwise there is redundant code in about eight places.

You mentioned Apple's doc style and they do something similar to what I did:

https://developer.apple.com/documentation/xcode/supporting-universal-links-in-your-app#Update-your-app-delegate-to-respond-to-a-universal-link

This example code shows how to handle a universal link in iOS and tvOS:

They show example code of what the customer's "deep link handling" has to do. The main difference is that In my version, I chose to be more explicit by showing calls to viewcontroller and so forth. The reason I'm doing this is because customers have consistently shown confusion over what Adjust SDK does and what their app does (even after showing them this Apple page). For example, a common question is why an Adjust deep link opens the app, but the app doesn't navigate the user to the correct screen. This example code helps us delineate what Adjust does and what the customer's code is expected to do. Let me revise the example code to make it a bit simpler and also put a clearer separation with comments to make it clear which part is Adjust and which is non-Adjust and just an example.

Also, I can introduce an explanation of what DeeplinkHandler is earlier in the doc.

@fuatde
Copy link
Collaborator

fuatde commented Dec 6, 2024

@DaveMeadAdjust, can you pls add me and @bzavhorodskyi to the project: https://github.com/adjust/dev-docs/
So we can approve the SidePanel part of the PR

@bzavhorodskyi
Copy link
Collaborator

@DaveMeadAdjust, can you pls add me and @bzavhorodskyi to the project: https://github.com/adjust/dev-docs/ So we can approve the SidePanel part of the PR

I need just information on why we changed the sidebar logic and what this changes fixes

@DaveMeadAdjust
Copy link
Collaborator Author

@DaveMeadAdjust, can you pls add me and @bzavhorodskyi to the project: https://github.com/adjust/dev-docs/ So we can approve the SidePanel part of the PR

I don't have write access to this repo, so I don't think I can add anyone. I found out that the sidebar logic got messed up on my local repo because I cloned an older version of the repo that had a different file structure. As a result, I'm going to create a different branch for all these changes instead. The sidebar logic update I did is actually not necessary. The sidebar logic works in production as expected.

@bzavhorodskyi
Copy link
Collaborator

@DaveMeadAdjust sure, I think we shouldn't change sidebar logic until it doesn't break in production

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.

5 participants