The colours for the password indicator is hardcoded in JS. Let's move it into CSS so it's easy for themes to override.

The classes used should follow our SMACSS-like conventions.

CommentFileSizeAuthor
#7 move-password-indicator-colors-into-css-2321167-7.patch2.69 KBemma.maria
#7 Screen Shot 2014-08-15 at 09.16.08.png530.64 KBemma.maria
#6 move-password-indicator-colors-into-css-2321167-6.patch.txt2.69 KBtompagabor
#4 Strong-Red.png172.27 KBmgifford
#4 Good-Grey.png172.35 KBmgifford
#4 Fair-Turquose.png169.68 KBmgifford
#4 Weak-blue.png196.4 KBmgifford
#2 password-colors.png2.09 MBemma.maria
#2 move-password-indicator-colors-into-css-2321167-2.patch2.72 KBemma.maria

Comments

emma.maria’s picture

emma.maria
she/her
commented
Assigned: Unassigned » emma.maria
emma.maria’s picture

emma.maria
she/her
commented
Assigned: emma.maria » Unassigned
Status: Active » Needs review
StatusFileSize
new move-password-indicator-colors-into-css-2321167-2.patch2.72 KB
new password-colors.png2.09 MB

I added the different colour classes as states (is-weak etc) which are set based on the width of the indicator just as the hardcoded colours were originally.
I added a removeClass before the addClass to tidy up the classes in the markup and make sure only one is set at a time.

Screenshots

mgifford’s picture

mgifford
he/him
Primary language English
commented

Works well on SimplyTest.me - http://s78da2e4c91252e2.s2.simplytest.me

mgifford’s picture

mgifford
he/him
Primary language English
commented
StatusFileSize
new Weak-blue.png196.4 KB
new Fair-Turquose.png169.68 KB
new Good-Grey.png172.35 KB
new Strong-Red.png172.27 KB

Some examples...

lewisnyman’s picture

lewisnyman
he/him
commented

Seems like a good fix

+++ b/core/modules/user/user.js
@@ -58,7 +58,8 @@
+              .removeClass('is-weak is-fair is-good is-strong')

I wonder if there is a better way of doing this. Let's see if JS people chime in.

tompagabor’s picture

tompagabor commented
StatusFileSize
new move-password-indicator-colors-into-css-2321167-6.patch.txt2.69 KB

Yes, there is a shorten way without jQuery.
Just change 

+              .removeClass('is-weak is-fair is-good is-strong')
+              .addClass(result.indicatorClass);

to

+              [0].className = 'password-strength__indicator ' + result.indicatorClass;
emma.maria’s picture

emma.maria
she/her
commented
StatusFileSize
new Screen Shot 2014-08-15 at 09.16.08.png530.64 KB
new move-password-indicator-colors-into-css-2321167-7.patch2.69 KB

Switched out the jquery in favour of @tompagabor's solution and it works well, thanks for the suggestion!

Screenshot to show Strong only has one class and a CSS colour + patch

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Reviewed & tested by the community

I would rather not have unrelated class name hardcoded in the JS (because then we'll have to deal with it later on). Also #5/7 would remove any custom class on that element. Let's only deal with classes this script is responsible for.

#2 is RTBC. Not better way without handling the state and we really don't want to deal with state for a simple script like that.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed #2 - 0d510d7 and pushed to 8.0.x. Thanks!

  • alexpott committed 0d510d7 on 8.0.x 
    Issue #2321167 by emma.maria, tompagabor | LewisNyman: Move password...

Status: Fixed » Closed (fixed)

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

droplet’s picture

droplet commented 
+++ b/core/modules/user/user.js
@@ -58,7 +58,7 @@
               .css('width', result.strength + '%')

so why do we hard-coded strength then ?