Problem/Motivation

While testing this module that can add a password input in vertical tabs on Claro, I noticed that only the password confirmation would overflow because of the CSS: .password-confirm { max-width: 25rem; }
I don't understand why we would want that instead of 100%.
Notice that this also makes the help text (div #edit-pass--description) width limited, contrary to the other fields in the same user edit form.

Proposed resolution

On the parent we already have max-width of 25rem; If we give input fields, width: 100% then it is aligning with the password policy as well. I have attached a patch and after patch screenshot for reference. Please review

After Patch

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#9 3362898-9.patch929 bytesgauravvvv
#8 3362898-8.patch4.35 KBgauravvvv
#8 Screenshot 2023-07-07 at 9.07.25 AM.png160.61 KBgauravvvv
#7 image15.jpg146.24 KBathyamvidyasagar
#7 3362898_5.patch574 bytesathyamvidyasagar
#5 olivero.PNG23.09 KBVidushi Mehta
#5 after.PNG4.66 KBVidushi Mehta
#5 before.PNG5.14 KBVidushi Mehta
#5 password-strength-3362898.patch923 bytesVidushi Mehta

Comments

kopeboy created an issue. See original summary.

kopeboy’s picture

kopeboy
Primary language Italian
Location Milan
commented
Component: CSS » Claro theme
Priority: Normal » Minor
Status: Active » Needs review

Said CSS is actually in Claro (I haven't tested with other themes).

kopeboy’s picture

kopeboy
Primary language Italian
Location Milan
commented
Issue summary: View changes
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs review » Needs work

Thanks for the bug report @kopeboy! 😊

The .password-confirm selector is supposed to target the wrapper of the passwords but it looks like the class is also applied on the second password item. 🙈 I think the right fix here would be to remove the password-confirm class from the pass2 password input.

Vidushi Mehta’s picture

Vidushi Mehta commented
Status: Needs work » Needs review
StatusFileSize
new password-strength-3362898.patch923 bytes
new before.PNG5.14 KB
new after.PNG4.66 KB
new olivero.PNG23.09 KB

Added a patch which is targeting the password strength as mentioned by the @kopeboy, I've checked on Olivero theme, same issue is showing there as well. Added before & after screenshots of the claro theme and one screenshot for the Olivero.

@lauriii are we removing the class as it should not be repeated on the child selector? Or any other specific reason for that please confirm

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs issue summary update

IS needs to be updated with proposed solution in #4

athyamvidyasagar’s picture

athyamvidyasagar commented
StatusFileSize
new 3362898_5.patch574 bytes
new image15.jpg146.24 KB

The password field is in consistence compare to other fields. Now with this patch all the fields are rendering in 100%. Screenshot is attached.

gauravvvv’s picture

gauravvvv commented
Status: Needs work » Needs review
StatusFileSize
new Screenshot 2023-07-07 at 9.07.25 AM.png160.61 KB
new 3362898-8.patch4.35 KB

I have added a patch with different approach. In patch #5, we are limiting the password strength with 25ch. 

 .password-strength {
   margin-block: var(--progress-bar-spacing-size);
+  max-width: 25ch;
 }

On the parent we already have max-width of 25rem; If we give input fields, width: 100% then it is aligning with the password policy as well. I have attached a patch and after patch screenshot for reference. Please review

After Patch

gauravvvv’s picture

gauravvvv commented
StatusFileSize
new 3362898-9.patch929 bytes

Ignore patch #8, I have made all changes into #9.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Seems issue summary still needs to be updated please.

gauravvvv’s picture

gauravvvv commented
Issue summary: View changes
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs issue summary update +Needs Review Queue Initiative

Thanks

  • lauriii committed 4d30ef79 on 11.x 
    Issue #3362898 by Gauravvvv, Vidushi Mehta, athyamvidyasagar, kopeboy,...

  • lauriii committed 5e775aae on 10.1.x 
    Issue #3362898 by Gauravvvv, Vidushi Mehta, athyamvidyasagar, kopeboy,...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

The fix in #9 looks good and I confirmed it addresses the original issue. We could fix the issue related .password-confirm applying to multiple elements in another issue.

Committed 4d30ef7 and pushed to 11.x. Also cherry-picked to 10.1.x as this is a low risk bug fix to Claro. Thanks!

gauravvvv’s picture

gauravvvv commented

Opened a follow up issue for .password-confirm #3374124: Remove the password-confirm class from the pass2 password input

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.