Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3534 Password Toggle Text Variation #3827

Closed
wants to merge 40 commits into from
Closed

Conversation

bgoewert
Copy link

@bgoewert bgoewert commented Jan 8, 2023

https://core.trac.wordpress.org/ticket/3534

This a text variation of #3725 to address accessibility concerns and add support for various password manager implementations. The button is separated from the input while still keeping it inline — allowing for password managers which add icons directly to the input, in contrast to those that insert another element to add their respective icon. I did not include the icon, which is inconsistent with the other toggle buttons, so as to provide maximum input space on the setup-config.php form. But the icon can easily be added back to remain consistent.

separated_pwd-toggle_inactive
Figure 1: Separated text password toggle w/ LastPass extension - inactive

separated_pwd-toggle_inactive
Figure 2: Separated text password toggle w/ LastPass extension - active

As a consequence of adding display: flex to .wp-pwd, the layout for install.php and user-new.php was also affected. I've also added changes to address this. For user-edit.php, user-profile.js#L232 applies display: block directly to .wp-pwd and was not affected.

install_before_change
Figure 3: install.php before.

install_before_fix
Figure 4: install.php after change.

install_after_fix
Figure 5: install.php after fix.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

Brennan Goewert and others added 30 commits December 4, 2022 09:28
Need to use global search next time when renaming things.
…ton applies to if there are multiple fields
Brennan Goewert added 3 commits January 20, 2023 19:05
@audrasjb
Copy link
Contributor

Hello, there is a small conflict in steup-config.php since I committed [55110]

Conflicts:
	src/wp-admin/setup-config.php
@bgoewert
Copy link
Author

Thanks @audrasjb. I just merged trunk to address the conflict.

Gruntfile.js Outdated Show resolved Hide resolved
Copy link

@sabernhardt sabernhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding text to the button. I marked some specific lines in the files.

This PR still adds an icon-only button to the setup-config page on smaller screens, which would need the 2.5rem side padding behind the button (both on the right, even in RTL).

English:
Password field in English at 500 pixels wide

Arabic:
Password field in Arabic at 500 pixels wide


.wp-pwd button {
height: min-content;
width: 4.875rem;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed width buttons can be trouble in other languages.

Spanish overflows a little:
setup-config in Spanish

Malayalam overflows a lot:
setup-config in Malayalam


.wp-pwd button.pwd-toggle .dashicons {
position: relative;
top: 0.25rem

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing semicolon.

@@ -593,6 +604,10 @@ fieldset label,
opacity: 1;
}

.password-input-wrapper {
display: inline-block;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be inline-block at smaller sizes. It seems fine on setup-config.php, but it changes both the Installation and Profile screens.

Installation:
installation screen at 500 pixels wide

Profile:
profile screen at 500 pixels wide

@@ -140,12 +140,14 @@ function display_setup_form( $error = null ) {
<td>
<div class="wp-pwd">
<?php $initial_password = isset( $_POST['admin_password'] ) ? stripslashes( $_POST['admin_password'] ) : wp_generate_password( 18 ); ?>
<input type="password" name="admin_password" id="pass1" class="regular-text" autocomplete="new-password" spellcheck="false" data-reveal="1" data-pw="<?php echo esc_attr( $initial_password ); ?>" aria-describedby="pass-strength-result" />
<span class="password-input-wrapper">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A div could be set to inline-block, if appropriate, to avoid putting the password-strength div inside a span.

@bgoewert
Copy link
Author

Closed in r56008

@bgoewert bgoewert closed this Sep 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants