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

openExternalLink returns a { opened: boolean | null } result #7273

Merged

Conversation

HamzaAtDiscord
Copy link
Contributor

Edit the return type for openExternalLink. With this change the result of openExternalLink will be an object containing a boolean value opened.

If opened is true, then the link was opened by the user
If opened is false, then the link was not opened by the user

The lifetime of the promise is also changed. Previously the promise completed after opening the "Leaving Discord" dialog, but with this change the promise completes after the user selects an action in the dialog (visit the site, or go back). If the dialog does not appear because the site visit is automatically approved, then the promise returns { opened: true } following the link opening.

@HamzaAtDiscord HamzaAtDiscord requested a review from a team as a code owner November 15, 2024 01:32
@HamzaAtDiscord HamzaAtDiscord requested review from JustinBeckwith and removed request for a team November 15, 2024 01:32
Copy link
Contributor

@matthova matthova left a comment

Choose a reason for hiding this comment

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

Once the SDK is merged lgtm

@HamzaAtDiscord HamzaAtDiscord changed the title openExternalLink returns a { opened: boolean } result openExternalLink returns a { opened: boolean | null } result Nov 19, 2024
@@ -1335,6 +1335,14 @@ No scopes required
|----------|--------|
| url | string |

#### OpenExternalLinkResponse

> **Note:** `{ opened: null }` is returned on older Discord clients that do not report the result of the open link action.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
> **Note:** `{ opened: null }` is returned on older Discord clients that do not report the result of the open link action.
> warn
> `{ opened: null }` is returned on older Discord clients that do not report the result of the open link action.

Copy link
Contributor

Choose a reason for hiding this comment

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

is there an "earliest client" we can include that would have this, or just include an approximate date (clients from before November XX, 2024)? just so when this is present, it still makes sense in a month or two

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated with the warn and to include a rough date (dec 24)

@HamzaAtDiscord HamzaAtDiscord merged commit a9fced6 into main Nov 21, 2024
4 checks passed
@HamzaAtDiscord HamzaAtDiscord deleted the hamzahutchinson/change-open-external-link-response branch November 21, 2024 19:05
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