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

Added new React Guide for Authenticated Routes #1675

Merged
merged 6 commits into from
Apr 13, 2022
Merged

Conversation

ErikCH
Copy link
Contributor

@ErikCH ErikCH commented Apr 12, 2022

Description of changes

Add new authenticated routes guide

Issue #, if available

Asana
image

Checklist

  • PR description included
  • yarn test passes
  • Tests are updated
  • Relevant documentation is changed or added (and PR referenced)

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

@changeset-bot
Copy link

changeset-bot bot commented Apr 12, 2022

⚠️ No Changeset found

Latest commit: 085a3b8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@@ -0,0 +1 @@
## Coming Soon
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, let's focus on React, and I'll add guides to Angular and Vue in the future.

Comment on lines +20 to +23
<Image
alt="Example screenshot of the route guard application."
height={{ base: '25rem', medium: '25rem' }}
display="block"
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 checked this image looked ok on mobile.

@ErikCH ErikCH temporarily deployed to ci April 12, 2022 21:08 Inactive
@ErikCH ErikCH temporarily deployed to ci April 12, 2022 21:08 Inactive
@ErikCH ErikCH temporarily deployed to ci April 12, 2022 21:08 Inactive
@ErikCH ErikCH temporarily deployed to ci April 12, 2022 21:08 Inactive
@ErikCH ErikCH requested review from wlee221 and reesscot April 12, 2022 21:13
Comment on lines +195 to +206
// RequireAuth.js
import { useLocation, Navigate } from 'react-router-dom';
import { useAuthenticator } from '@aws-amplify/ui-react';

export function RequireAuth({ children }) {
const location = useLocation();
const { route } = useAuthenticator((context) => [context.route]);
if (route !== 'authenticated') {
return <Navigate to="/login" state={{ from: location }} replace />;
}
return children;
}
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 found this pattern in the React router dom guide.

Copy link
Contributor

@wlee221 wlee221 left a comment

Choose a reason for hiding this comment

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

Adding some initial feedbacks. Wording feedbacks are obviously subjective, so feel free to disagree 🙏

docs/src/pages/guides/guide.web.mdx Outdated Show resolved Hide resolved
docs/src/pages/guides/guide.web.mdx Outdated Show resolved Hide resolved
docs/src/pages/guides/guide.web.mdx Outdated Show resolved Hide resolved
This forces the entire application behind a login page.

Let's imagine you needed a web application where the login page was in it's own route, and that you needed a handful of selected routes
to only be accessed by logged in users.
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like there's a newline here, this should be removed and let markdown handle line breaks.

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 was creating my own line breaks so it wasn't one super long line. But maybe my editor will do it automatically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

VS code has "Toggle word wrap" option that I like to use for markdown files 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice! That works :)

Copy link
Contributor

Choose a reason for hiding this comment

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

oh that is so nice, how have I gone this long without knowing about that

@ErikCH ErikCH temporarily deployed to ci April 13, 2022 21:42 Inactive
@ErikCH ErikCH temporarily deployed to ci April 13, 2022 21:42 Inactive
@ErikCH ErikCH temporarily deployed to ci April 13, 2022 21:42 Inactive
@ErikCH ErikCH temporarily deployed to ci April 13, 2022 21:42 Inactive
Copy link
Contributor

@jacoblogan jacoblogan left a comment

Choose a reason for hiding this comment

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

LGTM, Great job this is really going to help customers. One suggestion would be to use the ExampleCode component just to keep formatting across our docs consistent and give customers the Copy button that comes with it.

@@ -56,7 +56,7 @@ const NavLink = ({
);
};

const Nav = (props) => (
const Nav = ({ platform, ...props }: any) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

any?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ill remove that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -223,6 +223,15 @@ export const META_INFO: MetaInfo = {
description:
'A Theme is a structured collection of design decisions that change the appearance of a UI library. An Amplify UI theme is a structured object of design tokens, breakpoints, and overrides. - Amplify UI',
},
'/guides': {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for contributing to meta! LOL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LGTM, Great job this is really going to help customers. One suggestion would be to use the ExampleCode component just to keep formatting across our docs consistent and give customers the Copy button that comes with it.

OK! I added in the ExampleCode and did a bit more formatting.

@ErikCH ErikCH temporarily deployed to ci April 13, 2022 23:03 Inactive
@ErikCH ErikCH temporarily deployed to ci April 13, 2022 23:03 Inactive
@ErikCH ErikCH temporarily deployed to ci April 13, 2022 23:03 Inactive
@ErikCH ErikCH temporarily deployed to ci April 13, 2022 23:03 Inactive
@ErikCH ErikCH merged commit 572b728 into main Apr 13, 2022
@ErikCH ErikCH deleted the feat-new-guides-auth branch April 13, 2022 23:19
zchenwei added a commit that referenced this pull request Apr 20, 2022
* Added new React Guide for Authenticated Routes (#1675)

* Began work on new guides for auth

* Added React guide

* Updated guide

* capitilization update

* Addex Example Wrapper

Co-authored-by: Erik Hanchett <ehhanche@amazon.com>

* chore: align typescript version at root devDependencies (#1686)

* Fixed issue with vue examples (#1688)

Co-authored-by: Erik Hanchett <ehhanche@amazon.com>

* chore: use tsup for dev build (#1690)

* feat(Authenticator): accept general JSX children (#1685)

* Allow non-function children in Authenticator

* Create six-bikes-develop.md

* Update six-bikes-develop.md

* Update packages/react/src/components/Authenticator/Router/index.tsx

Co-authored-by: Caleb Pollman <cpollman@amazon.com>

* Update packages/react/src/components/Authenticator/Router/index.tsx

* Use React.ReactNode and consolidate children type

* Reduce nested ternary

* Add fragments around children

* Fix comments

* Update .changeset/six-bikes-develop.md

Co-authored-by: Caleb Pollman <cpollman@amazon.com>

Co-authored-by: Caleb Pollman <cpollman@amazon.com>

* fix: update focus styling for switchField  (#1667)

* fix: update focus styling for switchField

Co-authored-by: Heather Buchel <buchel@amazon.com>

* chore: update required permissions for publish-latest (#1708)

* docs: add section on token definition structure (#1671)

* docs: add section on token definition structure

Co-authored-by: Heather Buchel <buchel@amazon.com>
Co-authored-by: Scott Rees <6165315+reesscot@users.noreply.github.com>

* chore: (docs) update tabs component demo (#1703)

* chore: (docs) update tabs component demo

Co-authored-by: Heather Buchel <buchel@amazon.com>

* fix(docs Authenticator): Update style authenticator (#1707)

* Updated docs for styling for authenticator

* fixed it so only React sees information about provider

Co-authored-by: Erik Hanchett <ehhanche@amazon.com>

* chore: updating docs build to respect yarn.lock (#1714)

* chore: updating docs build to respect yarn.lock

* chore: adding resolution to clear cache

* Update tests.yml (#1715)

* chore: removing dep resolution

Co-authored-by: William Lee <43682783+wlee221@users.noreply.github.com>

* docs: fixing controlled example bug (#1716)

* docs: fixing dark mode example (#1719)

* docs: update Pagination (#1695)

* refactoring main pagination demo

* more pagination examples

* pagination styling examples

* adjust styles

* demonstrate usePagination hook, and include Theme example for styling

* remove extra backticks

* remove empty code section

Co-authored-by: Joe Buono <joebuono@amazon.com>

* chore: update menu demo to new format (#1702)

* fix(docs): Fix anchor tags between pages (#1706)

* Split off tests to `test-main` and `test-prs` (#1697)

* Split off tests to `test-main` and `test-prs`

* Adjust workflow names

* Add comments

* Skip cache for docs

* chore(ui-react): add jest generated coverage directory to .gitignore (#1718)

* chore: Bump maplibre-gl-js-amplify dependency version (#1721)

* Bump maplibre-gl-js-amplify dependency version

* Create patch changeset for version bump

* chore(bump map libre) (#1722)

* Bump maplibre

* Added changeset

* Added yarn lock

Co-authored-by: Erik Hanchett <ehhanche@amazon.com>

* docs: updating Link docs (#1709)

* docs: updating Link docs

* chore: addressing comments

* docs(fix): remove Theme tab from demos (#1723)

* temporarily remove Theme tab from docs demos

* removed nullish coalescing operator

* small typo

Co-authored-by: Joe Buono <joebuono@amazon.com>

* chore(ui-react): type withAuthenticator (#1724)

* chore: fixing workflow not publishing to next tag (#1725)

* chore: fixing workflow not publishing to next tag

* chore: updating workflow

* chore: updating publish-geo

* chore: update view demo to new format (#1710)

* Version Packages (#1693)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* docs: updating placeholder docs (#1705)

* docs: updating placeholder docs

* docs: adding Theme section

* chore: addressing comments

Co-authored-by: Erik Hanchett <icystorm@gmail.com>
Co-authored-by: Erik Hanchett <ehhanche@amazon.com>
Co-authored-by: Scott Rees <6165315+reesscot@users.noreply.github.com>
Co-authored-by: William Lee <43682783+wlee221@users.noreply.github.com>
Co-authored-by: Caleb Pollman <cpollman@amazon.com>
Co-authored-by: Heather Buchel <hbuchel@gmail.com>
Co-authored-by: Heather Buchel <buchel@amazon.com>
Co-authored-by: Joe Buono <joebuono724@gmail.com>
Co-authored-by: Joe Buono <joebuono@amazon.com>
Co-authored-by: Shane Laymance <shane.laymance@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

4 participants