OPAC: Toggle the visibility of the “Show Password"/eye icon in the Login widget
I'm going to re-word this title, as I don't think it's quite accurate.
This is an interesting one. While the eye icon is familiar from my memory, it looks like a lot of major sites use "<checkbox> Show password" instead underneath the password box. I'd be inclined to follow that usage.
I've done a little version of this for OPACUserJS. It'll need a little tweak or two for the community I suspect, but hoping to post a patch later today...
Created attachment 173115 [details] [review] Bug 34778: Add a 'Show password' toggle when logging into the OPAC 1. Apply the patch 2. perl build-resources.PL 3. Go to http://localhost:8080/cgi-bin/koha/opac-main.pl 4. Type "ABC" into the "Password" box and click "Show password" 5. Note that the password is now visible. Click again to hide password. 6. Click "Log in to your account" 7. Type "123" into the "Password" box and click "Show password" 8. Note that the password is now visible. Click again to hide password. 9. Note that the password on the main screen wasn't affected by the "Show password" in the modal 10. Repeat the above process on http://localhost:8080/cgi-bin/koha/opac-user.pl
Created attachment 173310 [details] [review] Bug 34778: Add a 'Show password' toggle when logging into the OPAC 1. Apply the patch 2. perl build-resources.PL 3. Go to http://localhost:8080/cgi-bin/koha/opac-main.pl 4. Type "ABC" into the "Password" box and click "Show password" 5. Note that the password is now visible. Click again to hide password. 6. Click "Log in to your account" 7. Type "123" into the "Password" box and click "Show password" 8. Note that the password is now visible. Click again to hide password. 9. Note that the password on the main screen wasn't affected by the "Show password" in the modal 10. Repeat the above process on http://localhost:8080/cgi-bin/koha/opac-user.pl Signed-off-by: David Nind <david@davidnind.com>
"Show password" need to be a label for the checkbox, so that when you click on the label you can easily toggle the checkbox.
(In reply to Jonathan Druart from comment #6) > "Show password" need to be a label for the checkbox, so that when you click > on the label you can easily toggle the checkbox. Ah touché. I remember thinking about that, and then probably went with span to ease the styling when implementing this feature just via OpacUserJS. Will revisit this one...
Created attachment 174442 [details] [review] Bug 34778: (follow-up) convert label from 'span' to 'label' element Also uses the "for" attribute to uniquely point to the correct checkbox.
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc
(In reply to Emmanuel Bétemps from comment #9) > CONFLICT (content): Merge conflict in > koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc Thanks Emmanuel. Uploading a rebased version.
Created attachment 176485 [details] [review] Bug 34778: Add a 'Show password' toggle when logging into the OPAC 1. Apply the patch 2. perl build-resources.PL 3. Go to http://localhost:8080/cgi-bin/koha/opac-main.pl 4. Type "ABC" into the "Password" box and click "Show password" 5. Note that the password is now visible. Click again to hide password. 6. Click "Log in to your account" 7. Type "123" into the "Password" box and click "Show password" 8. Note that the password is now visible. Click again to hide password. 9. Note that the password on the main screen wasn't affected by the "Show password" in the modal 10. Repeat the above process on http://localhost:8080/cgi-bin/koha/opac-user.pl
Created attachment 176486 [details] [review] Bug 34778: (follow-up) convert label from 'span' to 'label' element Also uses the "for" attribute to uniquely point to the correct checkbox.
Created attachment 176501 [details] [review] Bug 34778: Add a 'Show password' toggle when logging into the OPAC 1. Apply the patch 2. perl build-resources.PL 3. Go to http://localhost:8080/cgi-bin/koha/opac-main.pl 4. Type "ABC" into the "Password" box and click "Show password" 5. Note that the password is now visible. Click again to hide password. 6. Click "Log in to your account" 7. Type "123" into the "Password" box and click "Show password" 8. Note that the password is now visible. Click again to hide password. 9. Note that the password on the main screen wasn't affected by the "Show password" in the modal 10. Repeat the above process on http://localhost:8080/cgi-bin/koha/opac-user.pl Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Created attachment 176502 [details] [review] Bug 34778: (follow-up) convert label from 'span' to 'label' element Also uses the "for" attribute to uniquely point to the correct checkbox. Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Hi, each step of the test plan is ok (please, note I tested on a sandbox) one thing (which wasn't asked) I looked at is 'What if I want to change my password ?' -> there is no 'show my password' option Could it be useful to work on that ?
Perhaps Opac self-registration could also include this option 'show my password' when the account is created. The syspref is PatronSelfRegistration.
(In reply to Emmanuel Bétemps from comment #16) > Perhaps Opac self-registration could also include this option 'show my > password' when the account is created. > The syspref is PatronSelfRegistration. We though of this because someone when creating their bugzilla account good locked out due to a password mistake. ^^" And Bugzilla doesn't have the feature proposed here for Koha. But it turns out the proposal here wouldn't have prevented the scenario that happened with bugzilla. Note *if* self registration is too different it might risk stalling this bug.
Created attachment 176505 [details] [review] Bug 34778: Add a 'Show password' toggle when logging into the OPAC 1. Apply the patch 2. perl build-resources.PL 3. Go to http://localhost:8080/cgi-bin/koha/opac-main.pl 4. Type "ABC" into the "Password" box and click "Show password" 5. Note that the password is now visible. Click again to hide password. 6. Click "Log in to your account" 7. Type "123" into the "Password" box and click "Show password" 8. Note that the password is now visible. Click again to hide password. 9. Note that the password on the main screen wasn't affected by the "Show password" in the modal 10. Repeat the above process on http://localhost:8080/cgi-bin/koha/opac-user.pl Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 176506 [details] [review] Bug 34778: (follow-up) convert label from 'span' to 'label' element Also uses the "for" attribute to uniquely point to the correct checkbox. Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Auth is well covered by selenium tests. It should be easy and could be useful to have a couple of tests to cover this change.
Created attachment 176703 [details] [review] Bug 34778: Add a selenium test prove -v t/db_dependent/selenium/authentication.t
Created attachment 176904 [details] [review] Bug 34778: Remove extra 'bootstrap' when loading the js asset Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 176905 [details] [review] Bug 34778: Remove the include file
What about this last patch? It feels better to not rely on an additional include file when it can be done using JS easily.
Also it would have been good to apply this to all password fields we have in Koha, not only the login screens at the OPAC.
Created attachment 176907 [details] [review] Bug 34778: Add a 'Show password' toggle when logging into the OPAC 1. Apply the patch 2. perl build-resources.PL 3. Go to http://localhost:8080/cgi-bin/koha/opac-main.pl 4. Type "ABC" into the "Password" box and click "Show password" 5. Note that the password is now visible. Click again to hide password. 6. Click "Log in to your account" 7. Type "123" into the "Password" box and click "Show password" 8. Note that the password is now visible. Click again to hide password. 9. Note that the password on the main screen wasn't affected by the "Show password" in the modal 10. Repeat the above process on http://localhost:8080/cgi-bin/koha/opac-user.pl Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Created attachment 176908 [details] [review] Bug 34778: (follow-up) convert label from 'span' to 'label' element Also uses the "for" attribute to uniquely point to the correct checkbox. Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Created attachment 176909 [details] [review] Bug 34778: Add a selenium test prove -v t/db_dependent/selenium/authentication.t Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Created attachment 176910 [details] [review] Bug 34778: Remove extra 'bootstrap' when loading the js asset Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Created attachment 176911 [details] [review] Bug 34778: Remove the include file Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
(In reply to Jonathan Druart from comment #24) > What about this last patch? It feels better to not rely on an additional > include file when it can be done using JS easily. David, if you agree with this change please set to PQA.
(In reply to Jonathan Druart from comment #24) > What about this last patch? It feels better to not rely on an additional > include file when it can be done using JS easily. Sounds good to me. I like how you've written the JS there. (In reply to Jonathan Druart from comment #31) > (In reply to Jonathan Druart from comment #24) > > What about this last patch? It feels better to not rely on an additional > > include file when it can be done using JS easily. > > David, if you agree with this change please set to PQA. I don't know that I agree that the change was needed, but I've just tested and it works, so happy to PQA it.
(In reply to Jonathan Druart from comment #25) > Also it would have been good to apply this to all password fields we have in > Koha, not only the login screens at the OPAC. We've still got time to do that. What other screens do you have in mind for the OPAC? /cgi-bin/koha/opac-memberentry.pl is the only one that springs to mind at the moment for me, but I imagine there could be others.
Actually, it looks like the selenium test is failing now, so got to FQA it.
Created attachment 177272 [details] [review] Bug 34778: (follow-up) Remove the include file
Created attachment 177300 [details] [review] Bug 34778: Add a 'Show password' toggle when logging into the OPAC 1. Apply the patch 2. perl build-resources.PL 3. Go to http://localhost:8080/cgi-bin/koha/opac-main.pl 4. Type "ABC" into the "Password" box and click "Show password" 5. Note that the password is now visible. Click again to hide password. 6. Click "Log in to your account" 7. Type "123" into the "Password" box and click "Show password" 8. Note that the password is now visible. Click again to hide password. 9. Note that the password on the main screen wasn't affected by the "Show password" in the modal 10. Repeat the above process on http://localhost:8080/cgi-bin/koha/opac-user.pl Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 177301 [details] [review] Bug 34778: (follow-up) convert label from 'span' to 'label' element Also uses the "for" attribute to uniquely point to the correct checkbox. Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 177302 [details] [review] Bug 34778: Add a selenium test prove -v t/db_dependent/selenium/authentication.t Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 177303 [details] [review] Bug 34778: Remove extra 'bootstrap' when loading the js asset Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 177304 [details] [review] Bug 34778: Remove the include file Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 177305 [details] [review] Bug 34778: (follow-up) Remove the include file Signed-off-by: David Cook <dcook@prosentient.com.au>
Would love to see bug 24949 also in my queue for 25.05...
Pushed for 25.05! Well done everyone, thank you!