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

Reimplement forgot password for element-web 1.11.17 #373

Conversation

estellecomment
Copy link
Contributor

@estellecomment estellecomment commented Jan 16, 2023

Forgot password flow was changed a lot. The code is cleaner and the interface is easier to understand. But it caused a major patch break.
This PR reimplements the patch.


Screen Shot 2023-01-16 at 4 17 49 PM

----

Screen Shot 2023-01-16 at 4 19 11 PM

---

Screen Shot 2023-01-16 at 4 19 57 PM

---

Screen Shot 2023-01-16 at 4 22 03 PM

---

Screen Shot 2023-01-16 at 4 22 35 PM

---

Screen Shot 2023-01-16 at 4 26 23 PM

---

Screen Shot 2023-01-16 at 4 25 50 PM

---

Screen Shot 2023-01-16 at 4 26 50 PM

@odelcroi
Copy link
Member

It would be great to take into account this twin PR comment : #369 (comment)

@estellecomment
Copy link
Contributor Author

In english :

image

----

Screen Shot 2023-01-16 at 4 10 39 PM

----

Screen Shot 2023-01-16 at 4 16 41 PM

----

Screen Shot 2023-01-16 at 4 13 04 PM

@estellecomment
Copy link
Contributor Author

Text changes, non blockers but annoying

  • "Se connecter à la place" sounds weird to me, we can find better I'm sure
  • Title "Réinitialise le mot de passe" should be "Réinitialisez"
  • "Pas reçues" should be "Pas reçu"
  • do we use "e-mail", "email", "adresse mail", something else ?

@estellecomment
Copy link
Contributor Author

It would be great to take into account this twin PR comment : #369 (comment)

Not needed any more with new flow, the two texts are on different pages now.

@estellecomment
Copy link
Contributor Author

With this change, I think we should remove the warning text from the backend template completely.
Because

  • repeating the same information does not make people read more, it makes them feel spammed and read less. They have already read through a warning dialog.
  • if they went to click the link before continuing the flow, as instructed, then they will reach the backend template before they have had a change to chose whether to log out or not, so it will make no sense.

@odelcroi odelcroi mentioned this pull request Jan 16, 2023
43 tasks
@estellecomment
Copy link
Contributor Author

Going back to draft : I will fix the security regression in this PR, and resolve conflicts with the parent branch, and then it will be ready for review again.

@estellecomment
Copy link
Contributor Author

Done !
Fixes both issues described in #372
And removes an outdated bugfix patch.

@estellecomment estellecomment marked this pull request as ready for review January 19, 2023 11:15
@odelcroi
Copy link
Member

Text changes, non blockers but annoying

Wording has slighty changed indeed, non blocker can be fixed after. @Caroline-lawson what do you think?

Capture d’écran 2023-01-20 à 10 41 36

@odelcroi
Copy link
Member

odelcroi commented Jan 20, 2023

logout tested on :

  • browser
  • mobile needed?

@estellecomment
Copy link
Contributor Author

Filed an issue for the text changes @Caroline-lawson : #384

@estellecomment estellecomment merged commit 3fe4d78 into upgrade/element-web-v1.11.17 Jan 20, 2023
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.

3 participants