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

Improve password confirmation dialog #605

Conversation

@JuliaKirschenheuter JuliaKirschenheuter added the 2. developing Work in progress label Oct 5, 2023
@JuliaKirschenheuter JuliaKirschenheuter self-assigned this Oct 5, 2023
@JuliaKirschenheuter JuliaKirschenheuter changed the title Upgrade @nextcloud/vue lib and improve password confirmation dialog Improve password confirmation dialog Oct 5, 2023
src/components/PasswordDialog.vue Outdated Show resolved Hide resolved
@ShGKme
Copy link
Contributor

ShGKme commented Oct 5, 2023

Looks a little bit different than in the linked comment, but seems to be fine in terms of a11y

cc @nimishavijay

image

@ShGKme
Copy link
Contributor

ShGKme commented Oct 5, 2023

Upgrade to @nextcloud/vue@8 doesn't work without #606

@JuliaKirschenheuter
Copy link
Contributor Author

Looks a little bit different than in the linked comment,

what look s different?

@ShGKme
Copy link
Contributor

ShGKme commented Oct 5, 2023

what look s different?

Label position

Nimisha's comment Current view
image image

@JuliaKirschenheuter
Copy link
Contributor Author

what look s different?

Label position

Recording.2023-09-27.182535.mp4

@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/37080-update_nextcloud-vue_and_password_dialog branch from 101de3e to 5bc2765 Compare October 5, 2023 17:00
@JuliaKirschenheuter JuliaKirschenheuter marked this pull request as ready for review October 5, 2023 17:06
@ShGKme
Copy link
Contributor

ShGKme commented Oct 5, 2023

Is this problem in the comment from the linked issue also supposed to be fixed?

Should we do something with this message with failed authentication?

@ShGKme
Copy link
Contributor

ShGKme commented Oct 5, 2023

P.S. Don't forget to run

npm run l10n:extract

@JuliaKirschenheuter
Copy link
Contributor Author

@nickvergessen could i leave an error message like this

errorText: t('Failed to authenticate, your password was wrong'),
? There is only one input field with password confirmation so i think it is ok to have more specific message. Please see nextcloud/server#37080 (comment)

@ShGKme
Copy link
Contributor

ShGKme commented Oct 6, 2023

What about using NcPasswordField's error state instead of a large message?
https://nextcloud-vue-components.netlify.app/#/Components/NcFields?id=ncpasswordfield

image

Like on the Login page:

image

Then it would be:

  • clear that this message is about the entered password
  • more accessible because of aria-describedby in the field
  • no so changeable in the height size
. .
image image

@JuliaKirschenheuter JuliaKirschenheuter marked this pull request as draft October 9, 2023 09:22
@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/37080-update_nextcloud-vue_and_password_dialog branch 2 times, most recently from 467bdeb to 1c6fca7 Compare October 9, 2023 15:49
@JuliaKirschenheuter JuliaKirschenheuter marked this pull request as ready for review October 9, 2023 15:50
Signed-off-by: julia.kirschenheuter <[email protected]>
@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/37080-update_nextcloud-vue_and_password_dialog branch from 606d7e7 to b8b339c Compare October 9, 2023 16:35
@ShGKme ShGKme requested review from Pytal, susnux and artonge and removed request for susnux, artonge and Pytal October 9, 2023 16:51
src/components/PasswordDialog.vue Show resolved Hide resolved
@JuliaKirschenheuter JuliaKirschenheuter merged commit 695dc89 into master Oct 9, 2023
5 checks passed
@JuliaKirschenheuter JuliaKirschenheuter deleted the fix/37080-update_nextcloud-vue_and_password_dialog branch October 9, 2023 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress
Projects
None yet
5 participants