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

feat: redirect to referrer page after login #249

Merged
merged 13 commits into from
Jun 13, 2023
Merged

feat: redirect to referrer page after login #249

merged 13 commits into from
Jun 13, 2023

Conversation

huai-jie
Copy link
Contributor

@huai-jie huai-jie commented Jun 2, 2023

i.e

Clicking on the comment button on an item's page initiates the login process. It would be beneficial if, after logging in, users could be redirected back to the same item page.

@iuioiua
Copy link
Contributor

iuioiua commented Jun 5, 2023

Interesting means of redirecting... I like it! You might be able to extract both code blocks into a single combined middleware function and apply this to the /submit route. Can you do that?

@huai-jie huai-jie marked this pull request as draft June 5, 2023 11:57
@huai-jie
Copy link
Contributor Author

huai-jie commented Jun 5, 2023

You might be able to extract both code blocks into a single combined middleware function and apply this to the /submit route.

Hmm ...

Currently , I handle the cases for /submit , /account , and /[id] by adding the ?from= query into the redirect() function

i.e. redirect(/login?from=submit)

Then set the redirectUrl cookie accordingly in middleware

Finally, at /login/callback , I get the redirecturl from cookie and then redirect to it

I don't see if it needs to combine these 2 into a function

Then set the redirectUrl cookie accordingly in middleware
Finally get the redirecturl from cookie and then redirect to it at /login/callback

Still a bit confused about this. Would you mind providing more details or examples?

@huai-jie huai-jie marked this pull request as ready for review June 5, 2023 14:58
@iuioiua
Copy link
Contributor

iuioiua commented Jun 7, 2023

Deno SaaSKit aims to be modular. Having the newly introduced functionality from this PR encapsulated in a single function would be ideal, perhaps using pathnames: string[] as a parameter. If you don't think this is feasible, fair enough. My opinion is not strong. WDYT?

@huai-jie
Copy link
Contributor Author

huai-jie commented Jun 7, 2023

Okay, I believe it's worth a try to implement this improvement.

Will try to do this whenever I am free.

@huai-jie huai-jie marked this pull request as draft June 7, 2023 11:25
@iuioiua
Copy link
Contributor

iuioiua commented Jun 9, 2023

@huai-jie, can you do this for /account too? See #254 for context.

@huai-jie
Copy link
Contributor Author

huai-jie commented Jun 9, 2023

@huai-jie, can you do this for /account too? See #254 for context.

Sure 👌

@huai-jie huai-jie marked this pull request as ready for review June 9, 2023 11:21
Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

Could you please merge @/utils/http.ts into the new @/utils/redirect.ts along with tests?

utils/redirect.ts Outdated Show resolved Hide resolved
utils/redirect.ts Outdated Show resolved Hide resolved
utils/redirect.ts Outdated Show resolved Hide resolved
utils/redirect.ts Outdated Show resolved Hide resolved
@huai-jie huai-jie marked this pull request as draft June 12, 2023 06:31
@huai-jie huai-jie marked this pull request as ready for review June 13, 2023 10:30
Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

Great work, @huai-jie! Well done.

utils/redirect.ts Outdated Show resolved Hide resolved
@iuioiua iuioiua changed the title feat: redirect to referer page after login feat: redirect to referrer page after login Jun 13, 2023
@iuioiua iuioiua merged commit b0f5bb5 into denoland:main Jun 13, 2023
@huai-jie huai-jie deleted the redirect-to-referer branch June 13, 2023 11:39
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.

2 participants