WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 22 months ago

Last modified 22 months ago

#33167 closed defect (bug) (fixed)

Reset password screen lacks the Hide button and Confirm use of weak password checkbox

Reported by: johnbillion Owned by: adamsilverstein
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.3
Component: Login and Registration Keywords: has-patch has-screenshots commit
Focuses: ui Cc:

Description

When resetting my password, the "Hide" button isn't available, and the "Confirm use of weak password" checkbox isn't shown if I enter a weak password.

https://i.imgur.com/52BGcEM.png

Attachments (13)

33167.diff (3.2 KB) - added by manolis09 4 years ago.
Patch
Screen Shot 2015-09-20 at 16.28.54.png (75.9 KB) - added by manolis09 4 years ago.
Screenshot
33167.2.diff (2.4 KB) - added by umesh.nevase 4 years ago.
Added patch with UI Fix
33167.3.diff (3.3 KB) - added by umesh.nevase 4 years ago.
refreshed patch with 'hide/show' toggle button stylling
33167.4.diff (4.4 KB) - added by umesh.nevase 4 years ago.
Patch with responsive styling
33167.5.diff (4.4 KB) - added by umesh.nevase 4 years ago.
Patch with responsive styling
331767.6.diff (3.7 KB) - added by Nikschavan 4 years ago.
Refresh of 33167.5.diff
33167.7.diff (2.6 KB) - added by Nikschavan 4 years ago.
33167.6.diff (2.7 KB) - added by adamsilverstein 3 years ago.
33167.7.2.diff (2.8 KB) - added by Nikschavan 3 years ago.
vertically centeresthe show/hide dashicons
before.png (33.0 KB) - added by johnbillion 2 years ago.
after.png (33.4 KB) - added by johnbillion 2 years ago.
33167.8.diff (2.5 KB) - added by adamsilverstein 2 years ago.

Download all attachments as: .zip

Change History (41)

#1 follow-up: @markjaquith
4 years ago

Gonna say this isn't a blocker. Can revisit in 4.4, unless a heroic patcher wants to emerge really soon.

#2 in reply to: ↑ 1 @obenland
4 years ago

  • Milestone changed from 4.3 to Future Release

Replying to markjaquith:

Can revisit in 4.4, unless a heroic patcher wants to emerge really soon.

Yes, let's do that.

#3 @johnbillion
4 years ago

#33870 was marked as a duplicate.

#4 @johnbillion
4 years ago

  • Keywords ux-feedback removed
  • Milestone changed from Future Release to 4.4
  • Owner set to markjaquith
  • Status changed from new to assigned

#5 @manolis09
4 years ago

I've made some changes in CSS, JS and wp-login.php to make it work, however I didn't have time to work on the UI/UX part.

@manolis09
4 years ago

Patch

@umesh.nevase
4 years ago

Added patch with UI Fix

#6 @umesh.nevase
4 years ago

  • Keywords has-patch added; needs-patch removed

@umesh.nevase
4 years ago

refreshed patch with 'hide/show' toggle button stylling

@umesh.nevase
4 years ago

Patch with responsive styling

@umesh.nevase
4 years ago

Patch with responsive styling

#7 follow-up: @DrewAPicture
4 years ago

  • Focuses ui added
  • Keywords needs-refresh added
  • Owner changed from markjaquith to adamsilverstein

@umesh.nevase: Can you please refresh the patch from the WordPress root, that is to say, from within /trunk?

@adamsilverstein: Any chance you could review the approach here? Obviously in lieu of an applyable patch, that might be slow going, but worth a shot :-)

#8 in reply to: ↑ 7 @adamsilverstein
4 years ago

Sure, I will take a look.

Replying to DrewAPicture:

@adamsilverstein: Any chance you could review the approach here? Obviously in lieu of an applyable patch, that might be slow going, but worth a shot :-)

#9 @adamsilverstein
4 years ago

@johnbillion what device are you seeing this on? When I test at small screen size the show/hide feature moves inside the field. I also still see the confirm weak password field.

@Nikschavan
4 years ago

Refresh of 33167.5.diff

@Nikschavan
4 years ago

#11 @Nikschavan
4 years ago

  • Keywords has-screenshots added

In 33167.7.diff I am skipping the text 'show' from Show password button and aligning it inside input field.

Screenshots -

http://goo.gl/jTRhc5http://goo.gl/z4aBqy

#12 @Nikschavan
4 years ago

  • Keywords needs-refresh removed

#13 follow-up: @adamsilverstein
4 years ago

@Nikschavan:

Thanks for the patches!

This is looking really good, tested down to IE8.

I still see a few issues:

  • For the user edit screen, we settled on a password length of 24, I think we should use the same length here. Make the interface a bit more responsive or adjust CSS to allow for a longer string.
  • We should adjust CSS so users with JavaScript disabled don't get the unusable UI elements (screenshot with JS disabled - http://cl.ly/image/2v2s2e1m1J3J
  • I like the creative use of $(passStrength).is, however we generally try to avoid touching code outside the scope of the ticket and this seems unrelated.

#14 in reply to: ↑ 13 ; follow-up: @Nikschavan
4 years ago

Thank you for the suggestions @adamsilverstein, I will update the patch soon.

I do have some questions, though -

For the user edit screen, we settled on a password length of 24, I think we should use the same length here. Make the interface a bit more responsive or adjust CSS to allow for a longer string.

I quite did not get this part, I think for this to make it that will require increasing the width of the input field and overall width of the login form?

I like the creative use of $(passStrength).is, however, we generally try to avoid touching code outside the scope of the ticket and this seems unrelated.

This was added because on the login screen the #pass-strength-result has extra class .hide-if-no-js than on the user edit screen, because of this if ( 'short' === passStrength.className || 'bad' === passStrength.className ) condition is failing, so the new addition there.

On the UI part, How about adding the border from the input field around the input field and also the button?

http://screenshots.sharkz.in/nik/2015/11/2015-11-11_00-59-06.png

This will complicate stuff a bit though as same JS and CSS is reused for user edit page, I can still work on the patch.

Let me know your thoughts.

Last edited 4 years ago by Nikschavan (previous) (diff)

#15 in reply to: ↑ 14 @adamsilverstein
4 years ago

Replying to Nikschavan:

Let me know your thoughts.

I now wonder why this screen is so narrow? if we had a more responsive design here we could use the same layout and buttons as we do on other password reset screens.

This ticket was mentioned in Slack in #core by wonderboymusic. View the logs.


4 years ago

#17 @ocean90
4 years ago

  • Keywords needs-refresh added
  • Milestone changed from 4.4 to Future Release

#18 @ocean90
4 years ago

#34989 was marked as a duplicate.

#19 @ocean90
3 years ago

#38569 was marked as a duplicate.

#20 follow-up: @adamsilverstein
3 years ago

  • Keywords needs-refresh removed

33167.6.diff refreshed and updated patch; increased field size so longer password visible, reduced margin to give a little more room.

looks like this:

https://cl.ly/271r1n3E0o2r/Screen%20Recording%202016-12-26%20at%2012.52%20PM.gif

#21 @Nikschavan
3 years ago

Tested the patch and works perfectly.

Just one thing - The show/hide password is not in the center aligned for the desktop view -
https://cldup.com/ZMlyUSeFQT.jpg

Attaching a patch which center aligns the show/hide icon.

@Nikschavan
3 years ago

vertically centeresthe show/hide dashicons

#22 @adamsilverstein
2 years ago

  • Keywords commit added

@johnbillion
2 years ago

@johnbillion
2 years ago

#23 in reply to: ↑ 20 ; follow-up: @johnbillion
2 years ago

  • Keywords commit removed

Replying to adamsilverstein:

reduced margin to give a little more room.

This has had the side effect of making the regular login form look less aesthetically pleasing. See screenshots above.

#24 in reply to: ↑ 23 @adamsilverstein
2 years ago

  • Keywords dev-feedback added

This has had the side effect of making the regular login form look less aesthetically pleasing. See screenshots above.

@johnbillion thanks for catching that, I removed that change in 33167.8.diff . can you please give this one more review pass and let me know what you think? Thanks.

#25 @adamsilverstein
2 years ago

  • Keywords reporter-feedback added; dev-feedback removed

#26 @Nikschavan
22 months ago

The patch still applies cleanly against the trunk, Can this be targeted for 4.9?

#27 @adamsilverstein
22 months ago

  • Keywords commit added; reporter-feedback removed
  • Milestone changed from Future Release to 4.9

The patch still applies cleanly against the trunk, Can this be targeted for 4.9?

Yes, lets get this in!

#28 @adamsilverstein
22 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed in [41556].

Login: Password reset - add hide icon & confirm weak password checkbox.

Extends the password features added in 4.3 to the password reset flow.

Props johnbillion, manolis09, umesh.nevase, Nikschavan.

Last edited 22 months ago by adamsilverstein (previous) (diff)
Note: See TracTickets for help on using tickets.