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.

Sign up for GitHub

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

Jump to bottom

Redirect if authenticated, on pages reset, register, forgot #113

Merged
merged 11 commits into from Mar 25, 2020
Merged

Redirect if authenticated, on pages reset, register, forgot #113

merged 11 commits into from Mar 25, 2020

Conversation

deden
Copy link

@deden deden commented Feb 22, 2020

No description provided.

benjie
benjie reviewed Feb 26, 2020
View reviewed changes
Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

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

We definitely want this feature; but I think it might be better to achieve it with a prop on SharedLayout... Maybe something like <SharedLayout forbidWhen={LOGGED_IN | LOGGED_OUT | NEVER} (defaults to NEVER). What do you think?

@app/client/src/pages/forgot.tsx Outdated Show resolved Hide resolved
@deden
Copy link
Author

deden commented Feb 28, 2020

Sounds good to me. I've tried implementing it in SharedLayout prop with a AuthRestrict enum.
what do you think?

benjie
benjie requested changes Feb 28, 2020
View reviewed changes
Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

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

This is looking good. 👍

@app/client/src/layout/SharedLayout.tsx Outdated Show resolved Hide resolved
@benjie
Copy link
Member

benjie commented Mar 25, 2020

I've merged master and added the LOGGED_OUT branch. Merging after organizations-1 was non-trivial so I thought it fairer if I did it myself 😅

@benjie
Copy link
Member

benjie commented Mar 25, 2020

Reviewed all <SharedLayout uses and enhanced a few more behaviours... I think I co-opted this PR a little 😆

benjie
benjie approved these changes Mar 25, 2020
View reviewed changes
Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

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

Thanks! 🙌

@benjie benjie merged commit 0c8e319 into graphile:master Mar 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjie benjie benjie approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@deden @benjie