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

chore: add information about the new signOut redirect url option #7938

Merged

Conversation

Samaritan1011001
Copy link
Contributor

@Samaritan1011001 Samaritan1011001 commented Sep 4, 2024

Description of changes:

Adds two similar sections on using the new parameter on signOut API to both React Native and Web filters to the "Add social provider sign-in" section of the documentation. This change is to support the feature release in Amplify JS.

Related GitHub issue #, if available:

Instructions

If this PR should not be merged upon approval for any reason, please submit as a DRAFT

Which product(s) are affected by this PR (if applicable)?

  • amplify-cli
  • amplify-ui
  • amplify-studio
  • amplify-hosting
  • amplify-libraries

Which platform(s) are affected by this PR (if applicable)?

  • JS
  • Swift
  • Android
  • Flutter
  • React Native

Please add the product(s)/platform(s) affected to the PR title

Checks

  • Does this PR conform to the styleguide?

  • Does this PR include filetypes other than markdown or images? Please add or update unit tests accordingly.

  • Are any files being deleted with this PR? If so, have the needed redirects been created?

  • Are all links in MDX files using the MDX link syntax rather than HTML link syntax?

    ref: MDX: [link](https://docs.amplify.aws/)
    HTML: <a href="https://docs.amplify.aws/">link</a>

When this PR is ready to merge, please check the box below

  • Ready to merge

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Samaritan1011001 Samaritan1011001 requested a review from a team as a code owner September 4, 2024 20:52
@Samaritan1011001 Samaritan1011001 changed the title chore: add signOut redirect url option example section to relevant parts of the docs chore: add information about signOut redirect url option to relevant parts of the docs Sep 4, 2024
@Samaritan1011001 Samaritan1011001 changed the title chore: add information about signOut redirect url option to relevant parts of the docs chore: add information about the new signOut redirect url option Sep 4, 2024
Copy link
Member

Choose a reason for hiding this comment

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

Sanity check: Gen2 doc doesn't have an equivalent page for adding this information?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find any relevant place. Only relatable location is the signOut page but that's for the common use-case and Gen2 has nothing on signInWithRedirect I believe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added it here for Gen2.

Copy link
Member

@jimblanc jimblanc left a comment

Choose a reason for hiding this comment

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

Few nits but looks good!

Copy link
Member

@israx israx left a comment

Choose a reason for hiding this comment

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

I think we should move the proposed changes to the signOut section on gen2 docs. Eventually, gen1 docs will be deprecated, requiring us to make changes.

@@ -739,6 +739,74 @@ function App() {
</Block>
</BlockSwitcher>

### Redirect URLs
Copy link
Member

Choose a reason for hiding this comment

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

I think this section is redundant. By examining lines 744 and 746, I interpret that the change we are currently implementing is the inclusion of multiple redirects, which was actually a feature offered by v6 upon its release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On RN page, we don't have any info on redirects so I thought this will set up some context before diving into multiple redirects.

jimblanc
jimblanc previously approved these changes Sep 5, 2024
israx
israx previously approved these changes Sep 6, 2024
HuiSF
HuiSF previously approved these changes Sep 6, 2024
Copy link
Contributor

@josefaidt josefaidt left a comment

Choose a reason for hiding this comment

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

left a few comments but overall we'll want to consolidate the snippets to a common pattern (ts, minimal usage examples), add a callout to the sign-out page for when to use this, and add a short explainer on when or why you'd want to use this feature

@Samaritan1011001 Samaritan1011001 merged commit 6c6ce5c into aws-amplify:main Sep 10, 2024
12 checks passed
@Samaritan1011001 Samaritan1011001 deleted the feat/signout-redirect branch September 10, 2024 23:17
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.

6 participants