Add a "show password" button in forms#9490
Add a "show password" button in forms#9490microstudi wants to merge 10 commits intodecidim:developfrom
Conversation
|
It seems like you didn't give us much information about what you're trying to do here. We would appreciate it if you could provide us with more info about this issue/PR! |
|
This PR is ready for review @andreslucena @ahukkanen @furilo @virgile-dev Does this means that it is still useful? We have other 3 PR pending that tackles the user registrations, all of them accepted by product not long ago but, at this point we feel uncertain on how to proceed on all of them. BTW, It's been said that any new contribution has to follow the new redesign but, how we do that if the new design is not done yet? do we have html/css guide lines already? or does this means that any new contribution has to wait until the rework done by the contractor is (almost) finished? |
|
It's all good on our end with Simonas. Thanks @microstudi |
|
Hi @microstudi @virgile-dev
On this case, I'm afraid you should wait until we have merged #9455, then rebase/merge against Mind that the plan now is to have v0.28 with the new design already. Sorry for the problems that this could mean to you guys. |
|
Hello @andreslucena I’m honestly surprised by your message. We’ve been quite thorough with how we approached this contribution. We’ve done extensive research (benchmark competitors, user tests, defined KPIs for success) to then publish a series of proposals with a precise scope. We’ve waited for product’s greenlight, when asked what our intentions were in term of timelines we were clear that we were aiming the next release. Again we got a green light and launched a collaboration with Ivan and Pau to get the PR ready fast to avoid creating a delay in 0.27 release. Though I understand the redesign guidelines post 0.27 release, I’m puzzled by your message here, which goes against our previous communication with the product team. If a coordination mistake was made it’s okay but we would have appreciated you take the time to jump into a call with us to explain to us what happened. Now we have to rescope everything into a module that will have only a 6 months utility. It implies unforeseen cost for us and our clients and blocks Ivan and Pau on a project they were planning on shipping quickly. This goes without saying these improvements which are mostly UX, not UI, will need to be recoded in the middle term future. Sounds like a waste of time for everyone. I really hope that in the future we can have better communication between all parties and avoid creating such frustrations which make for a poor contributing experience. |
|
Sorry to hear that @virgile-dev. I have to say that these proposals were superb, and we really wanted/want to see them implemented. I think that we need to analyze deeply what happened here. In particular, I think that we (product and the whole community) have to start thinking about what goes on in the main repository as a really slow process. We try to maintain a minimum on quality, and that doesn't play nice with deadlines. If you have your own commitments and calendar, please don't put that pressure on the project. Just to understand with an example that anyone can control the timings: one year ago, Barcelona had to make some developments related to the participatory budgeting. As we've seen that it was impossible to make the code, the review, and the release on time, we've forked decidim and worked on that fork just for this process (see AjuntamentdeBarcelona/decidim-barcelona#368). This is not to put excuses, I repeat that I personally think that we need to analyze thoroughly what happened here. But it's important that everyone understand that the processes of decidim are slow, and if you have a deadline, then you need to find a solution that doesn't involve the coding/reviewing/merging/releasing process. These alternatives are well-supported by decidim: forking, making a module, or making overrides at the application level. |
|
I just wanted to note here that this should wait for #9455 to be completed and merged first as otherwise it's going to be pretty hard implementing this because these pages haven't been redesigned yet. |
|
As this looks it won't be merged, we've created a new module for this an other features. Meanwhile we are closing this, feel free to reuse some of this code in the future if this feature is still relevant then. |
Comes from #9490 Made by @microstudi
Comes from #9490 Made by @microstudi
Comes from #9490 Made by @microstudi
* Remove password confirmation field * Bring initial implementation for PasswordToggler JS class Comes from #9490 Made by @microstudi * Adapt PasswordToggler to redesign and call it * Change the callers of password_field method so they all use the partial * Add spec * Migrate PasswordToggler to vanilla JS * Fix specs * Fix accessibility errors * Refactor and clean-up javascript code * Don't wrap the password label with the input and the button * Fix linter offense * Fix bug when there's an error message * Prevent submission on enter * Show label with required indicator (asterisk and a11y goodies) * Fix button paddings when there's an error * Fix failing spec when not showing help text on failed password reset * Show required password on new session form * Clean-up `password_required?` method Suggested by code review Co-authored-by: Alexandru Emil Lupu <contact@alecslupu.ro> --------- Co-authored-by: Alexandru Emil Lupu <contact@alecslupu.ro>
🎩 What? Why?
This PR adds a password toggle button via unobstructive javascript (defaults to the traditional password/confirm password if javascript is disabled). This means it also hides the "confirm password" field.
To ensure full accessibility, it is based on GOV.UK article: https://technology.blog.gov.uk/2021/04/19/simple-things-are-complicated-making-a-show-password-option/
(see their implementation here: https://github.com/alphagov/govuk_publishing_components/blob/main/app/assets/javascripts/govuk_publishing_components/components/show-password.js)
Applies to:
System registrationSystem edit adminNOTE: While doing this I noticed this bug #9498 (I skipped modifying that obsolete view)
📌 Related Issues
Link your PR to an issue
Testing
Describe the best way to test or validate your PR.
📋 Checklist
🚨 Please review the guidelines for contributing to this repository.
docs/.📷 Screenshots
Please add screenshots of the changes you're proposing