[wp-trac] [WordPress Trac] #34376: Review the FTP credentials form
WordPress Trac
noreply at wordpress.org
Wed May 11 23:28:58 UTC 2016
#34376: Review the FTP credentials form
----------------------------+--------------------------------------------
Reporter: afercia | Owner: mte90
Type: defect (bug) | Status: assigned
Priority: normal | Milestone: 4.6
Component: Administration | Version: 4.2
Severity: normal | Resolution:
Keywords: has-patch | Focuses: ui, accessibility, javascript
----------------------------+--------------------------------------------
Comment (by afercia):
Thanks very much @Mte90. Some feedback on the patch.
My bad, maybe in my ticket description the words "above all the other
fields" are a bit confusing, I meant to move the "Connection Type" radio
buttons above the "Authentication Keys" fields but below the Username and
Password fields. I'd recommend this order:
- Hostname
- Username
- Password
- Connection Type
- and if SSH2 is selected: Authentication Keys
This way, when users change the selected radio buttons, the Authentication
Keys fields will collapse/expand '''below''' the radio buttons
The toggled element should include the span with the text "Enter the
location on the server where the public ..." so maybe wrap everything in a
div?
[[Image(https://cldup.com/foYgmVGutV.png)]]
[[Image(https://cldup.com/1jv2EAMIuP.png)]]
About the JS part, there are now both click and change events attached on
the radio buttons and they call functions meant to do the same thing
[[Image(https://cldup.com/bchVuFZpkL.png)]]
The `click` event comes from the old inline JS now moved in `updates.js`
{{{
jQuery("#ssh").click(function () {
jQuery("#ssh_keys").show();
});
jQuery("#ftp, #ftps").click(function () {
jQuery("#ssh_keys").hide();
});
jQuery('#request-filesystem-credentials-form
input[value=""]:first').focus();
}}}
The `change` event comes from
{{{
// Hide SSH fields when not selected
$( '#request-filesystem-credentials-dialog input[name="connection_type"]'
).on( 'change', function() {
$( this ).parents( 'form' ).find( '#private_key, #public_key'
).parents( 'label' ).toggle( ( 'ssh' == $( this ).val() ) );
}).change();
}}}
I'd keep the `change` event and in the first block keep just the line to
set initial focus on the first form field and remove all the rest.
The second block should be adjusted to toggle the entire fieldset and the
description text (it currently targets only the public and private fields
labels).
Also, it currently works only in the modal, the jQuery selector for `on()`
should be adjusted in order to work also in the Updates page. I'd probably
just remove `#request-filesystem-credentials-dialog` unless I'm missing
something.
Lastly, it would be a nice opportunity for some coding standards, but only
on the changed lines please :)
--
Ticket URL: <https://core.trac.wordpress.org/ticket/34376#comment:8>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list