Skip to content
This repository has been archived by the owner on Sep 10, 2024. It is now read-only.

Account Recovery: replace the server-side rendered form with an interactive React-based form #3016

Merged
merged 4 commits into from
Jul 26, 2024

Conversation

reivilibre
Copy link
Contributor

@reivilibre reivilibre commented Jul 25, 2024

Part of #172: the interactive form has a live-feedback meter for password complexity

TODO (would like your help please):

  • how do we redirect to the login form (non-React) after success? Maybe the right answer is just using vanilla JS navigation but would like your thoughts.

commit by commit :-)

@reivilibre reivilibre requested a review from sandhose July 25, 2024 22:15
@reivilibre reivilibre marked this pull request as ready for review July 25, 2024 22:16
Copy link

cloudflare-workers-and-pages bot commented Jul 25, 2024

Deploying matrix-authentication-service-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8e2c304
Status: ✅  Deploy successful!
Preview URL: https://de7e1254.matrix-authentication-service-docs.pages.dev
Branch Preview URL: https://rei-fe-pwrecovery.matrix-authentication-service-docs.pages.dev

View logs

@@ -6,7 +6,8 @@
"close": "Close",
"continue": "Continue",
"edit": "Edit",
"save": "Save"
"save": "Save",
"save_and_continue": "Save and continue"
Copy link
Member

Choose a reason for hiding this comment

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

The action.* namespace is shared with EW/EX, so I think we have to be slightly careful here. That key does not exist there yet, but it looks alright and fine to share. @t3chguy opinions?

Copy link
Member

Choose a reason for hiding this comment

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

if the text exists in Figma and seems a short reusable action such as this then I think its the right way to do it

response.data?.setPasswordByRecovery.status === SetPasswordStatus.Allowed
) {
// TODO HOW DO WE NAVIGATE
router.navigate({ to: "/../login" });
Copy link
Member

@sandhose sandhose Jul 26, 2024

Choose a reason for hiding this comment

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

Honestly window.location.href = '/login' might be fine. Part of the annoying thing here is getting the exact app root, as MAS might be mounted on a subpath.

Maybe the best we can do is load the react app root (aka /account) but with a full page load so that the backend takes care of redirecting us to /login, so something like

const router = useRouter(); // not in the if, because of the rules of hooks

const location = router.buildLocation({ to: "/" });
window.location.href = location.href;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup that works

@reivilibre reivilibre merged commit 48c4c34 into main Jul 26, 2024
13 checks passed
@reivilibre reivilibre deleted the rei/fe_pwrecovery branch July 26, 2024 09:20
@sandhose sandhose added the A-Local-Password Related to the local password database label Jul 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Local-Password Related to the local password database
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants