-
Notifications
You must be signed in to change notification settings - Fork 50
Deprecation docs/new functions for abort and isIdentityAvailable #781
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
Deprecation docs/new functions for abort and isIdentityAvailable #781
Conversation
genwhittTTD
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed and made some comments. But the updates need to be in the sdk-ref-javascript.md file, not this one which is for an earlier version.
docs/sdks/sdk-ref-javascript-v2.md
Outdated
|
|
||
| ### isIdentityAvailable(): boolean <New3100 /> | ||
|
|
||
| Specifies whether an identity is available. An identity can be available if there is an identity in local storage or a cookie that is not expired, or if there is an active request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest:
Determines whether an identity is available: for example, if there is an identity in local storage, a cookie that is still valid (not expired), or an existing active request.
Note -- I don't really understand the active request part. A request doesn't mean that it's available. Not sure if this is correct but maybe we could say whether a valid identity is available (for example, in local storage or in a cookie) or has already been requested.
docs/sdks/sdk-ref-javascript-v2.md
Outdated
|
|
||
| | Value | Description | | ||
| | :--- | :--- | | ||
| | `true` | This value indicates one of the following:<br/>- The identity is present and valid in a first-party cookie or local storage.<br/>- The identity has expired, and the token was not refreshed due to an intermittent error. The identity might be restored after a successful auto-refresh attempt. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe here we could include that the scenario means that the identity has expired but the refresh token is still valid... weave that in somehow?
docs/sdks/sdk-ref-javascript-v2.md
Outdated
| ### abort(): void | ||
| ### abort(): void <Deprecated3100 /> | ||
|
|
||
| This function is deprecated and will be removed altogether in June of 2025. Use [disconnect()](#disconnect-void) instead as it performs the same logic as `abort()` plus a more thorough disconnection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple minor wording suggestions. Your call.
This function is deprecated and support will be removed completely in June of 2025. Instead, use disconnect() which does the same thing and includes improved, more thorough disconnection logic.
genwhittTTD
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two broken links to fix... the rest looks great. Approved with links fixed.
| - [getAdvertisingToken()](#getadvertisingtoken-string) | ||
| - [getAdvertisingTokenAsync()](#getadvertisingtokenasync-promise) | ||
| - [isLoginRequired()](#isloginrequired-boolean) | ||
| - [isIdentityAvailable()](#isidentityavailable-boolean) <New /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a broken link. Also the other update.
I suggest that you don't mess with the headings but instead maybe, for those two functions, put the "new in" or "deprecated" on the next line, after the heading. For the other functions that have tags, they've been omitted in the body copy. I think it's good info to have with the content, but if you put it in the heading, because it's Markdown, you'd have to update the link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed with the latest update... all good, thx.
| - [isIdentityAvailable()](#isidentityavailable-boolean) <New /> | ||
| - [disconnect()](#disconnect-void) | ||
| - [abort()](#abort-void) | ||
| - [abort()](#abort-void) <Deprecated3100 /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above. Link is broken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good now. Thx.
genwhittTTD
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thx.
No description provided.