Repository navigation
Application password UX improvements #11079
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
c24eea0
395cd85
f569501
9d04cdc
24737a6
b417df5
bc01588
3aeaf3f
59eff7d
480360e
3483c27
42da2e2
91cc2fb
112720d
4455065
4680632
71af8dc
5ca5a6d
ab94164
010f07a
6d91d6d
2cb69eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,19 +2,50 @@ | |
| * @output wp-admin/js/auth-app.js | ||
| */ | ||
|
|
||
| /* global authApp */ | ||
| /* global authApp, ClipboardJS */ | ||
|
|
||
| ( function( $, authApp ) { | ||
| var $appNameField = $( '#app_name' ), | ||
| $approveBtn = $( '#approve' ), | ||
| $rejectBtn = $( '#reject' ), | ||
| $form = $appNameField.closest( 'form' ), | ||
| clipboard = new ClipboardJS( '.application-password-display .copy-button' ), | ||
| context = { | ||
| userLogin: authApp.user_login, | ||
| successUrl: authApp.success, | ||
| rejectUrl: authApp.reject | ||
| successUrl: authApp.success | ||
| }; | ||
|
|
||
| // Bound at module scope so it covers both the no-JS server-rendered button and the one inserted after the AJAX request. | ||
| clipboard.on( 'success', function( e ) { | ||
| var $successElement = $( '.success', $( e.trigger ).parent() ); | ||
|
|
||
| e.clearSelection(); | ||
| $successElement.removeClass( 'hidden' ); | ||
|
|
||
| setTimeout( function() { | ||
| $successElement.addClass( 'hidden' ); | ||
| }, 3000 ); | ||
|
|
||
| wp.a11y.speak( wp.i18n.__( 'Application password has been copied to your clipboard.' ) ); | ||
| } ); | ||
|
|
||
| // If redirecting to an external site, gate the approve button behind the confirmation checkbox. | ||
| if ( authApp.successHost ) { | ||
| var $checkbox = $( 'input[name="confirm_external_redirect"]' ); | ||
|
|
||
| // Start the approve button in a disabled state. | ||
| $approveBtn.prop( 'aria-disabled', true ).addClass( 'disabled' ); | ||
|
|
||
| // Toggle the approve button when the checkbox state changes. | ||
| $checkbox.on( 'change', function() { | ||
| if ( $checkbox.prop( 'checked' ) ) { | ||
| $approveBtn.removeProp( 'aria-disabled' ).removeClass( 'disabled' ); | ||
| } else { | ||
| $approveBtn.prop( 'aria-disabled', true ).addClass( 'disabled' ); | ||
| } | ||
| } ); | ||
| } | ||
|
|
||
| $approveBtn.on( 'click', function( e ) { | ||
| var name = $appNameField.val(), | ||
| appId = $( 'input[name="app_id"]', $form ).val(); | ||
|
|
@@ -25,8 +56,8 @@ | |
| return; | ||
| } | ||
|
|
||
| if ( 0 === name.length ) { | ||
| $appNameField.trigger( 'focus' ); | ||
| if ( ! $form[ 0 ].checkValidity() ) { | ||
| $form[ 0 ].reportValidity(); | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -44,12 +75,12 @@ | |
| * Filters the request data used to Authorize an Application Password request. | ||
| * | ||
| * @since 5.6.0 | ||
| * @since x.y.z A reject URL is no longer supported or used. | ||
| * | ||
| * @param {Object} request The request data. | ||
| * @param {Object} context Context about the Application Password request. | ||
| * @param {string} context.userLogin The user's login username. | ||
| * @param {string} context.successUrl The URL the user will be redirected to after approving the request. | ||
| * @param {string} context.rejectUrl The URL the user will be redirected to after rejecting the request. | ||
| */ | ||
| request = wp.hooks.applyFilters( 'wp_application_passwords_approve_app_request', request, context ); | ||
|
|
||
|
|
@@ -89,17 +120,25 @@ | |
| /* translators: %s: Application name. */ | ||
| '<label for="new-application-password-value">' + wp.i18n.__( 'Your new password for %s is:' ) + '</label>', | ||
| '<strong></strong>' | ||
| ) + ' <input id="new-application-password-value" type="text" class="code" readonly="readonly" value="" />'; | ||
| ); | ||
| $notice = $( '<div></div>' ) | ||
| .attr( 'role', 'alert' ) | ||
| .attr( 'tabindex', -1 ) | ||
| .addClass( 'notice notice-success notice-alt' ) | ||
| .append( $( '<p></p>' ).addClass( 'application-password-display' ).html( message ) ) | ||
| .append( | ||
| $( '<p></p>' ) | ||
| .addClass( 'application-password-display' ) | ||
| .append( '<input id="new-application-password-value" type="text" class="code" readonly="readonly" value="" />' ) | ||
| .append( ' <button type="button" class="button copy-button">' + wp.i18n.__( 'Copy' ) + '</button>' ) | ||
| .append( '<span class="success hidden" aria-hidden="true"> ' + wp.i18n.__( 'Copied!' ) + '</span>' ) | ||
| ) | ||
| .append( '<p>' + wp.i18n.__( 'Be sure to save this in a safe location. You will not be able to retrieve it.' ) + '</p>' ); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an existing issue, but this text should be connected to the password input via |
||
|
|
||
| // We're using .text() to write the variables to avoid any chance of XSS. | ||
| $( 'strong', $notice ).text( response.name ); | ||
| $( 'input', $notice ).val( response.password ); | ||
| $( '.copy-button', $notice ).attr( 'data-clipboard-text', response.password ); | ||
|
|
||
| $form.replaceWith( $notice ); | ||
| $notice.trigger( 'focus' ); | ||
|
|
@@ -147,16 +186,22 @@ | |
| * Fires when an Authorize Application Password request has been rejected by the user. | ||
| * | ||
| * @since 5.6.0 | ||
| * @since x.y.z A reject URL is no longer supported or used. | ||
| * | ||
| * @param {Object} context Context about the Application Password request. | ||
| * @param {string} context.userLogin The user's login username. | ||
| * @param {string} context.successUrl The URL the user will be redirected to after approving the request. | ||
| * @param {string} context.rejectUrl The URL the user will be redirected to after rejecting the request. | ||
| */ | ||
| wp.hooks.doAction( 'wp_application_passwords_reject_app', context ); | ||
|
|
||
| // @todo: Make a better way to do this so it feels like less of a semi-open redirect. | ||
| window.location = authApp.reject; | ||
| var $notice = $( '<div></div>' ) | ||
| .attr( 'role', 'alert' ) | ||
| .attr( 'tabindex', -1 ) | ||
| .addClass( 'notice notice-info' ) | ||
| .append( $( '<p></p>' ).text( wp.i18n.__( 'You have not approved this connection. No data has been shared with the application.' ) ) ); | ||
|
|
||
| $form.replaceWith( $notice ); | ||
| $notice.trigger( 'focus' ); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setting focus to a container that also has Since focus has to be changed, due to the removal of the form contents and current focus location, I'd recommend removing the alert role to prevent the duplicate announcement. |
||
| } ); | ||
|
|
||
| $form.on( 'submit', function( e ) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, but it makes the gate weaker than it looks: the approve button is disabled with
.prop( 'aria-disabled', true ).jQuery's
.prop()with a hyphenated, non-native name sets an expando property on the element rather than an attribute, so nothing is reflected into the DOM and assistive technology never sees the disabled state. The click guard further down then returns silently with no feedback..attr()/.removeAttr()throughout would make it real. Alternatively drop the manual disabled state entirely and lean on the existingreportValidity()call, which already surfaces the required-checkbox message natively.