Review the FTP credentials form

[afercia]

Note: to make the FTP credentials form appear and test it I'm using the ​Core Control plugin.

In the FTP credentials form there's still some inline JavaScript to toggle the visibility of an element with ID "ssh_keys". That element was a table row for the SSH authentication keys and the table was removed in [31949]. The SSH fields are still there, but not inside a table.

Checking how it worked in WordPress 4.0, the SSH fields were initially hidden with some inline CSS if 'ssh' != $connection_type and then the visibility toggled when clicking the FTP or FTPS radio buttons:

I think the outdated part of the inline JavaScript should be tweaked or better removed in favor of what's in updates.js that currently works only in the modal.

I'd recommend also to move the "Connection type" radio buttons above all the other fields: for accessibility the revealed elements should be after the control that revealed them.

[afercia]

I'd recommend also to move the "Connection type" radio buttons above all the other fields: for accessibility the revealed elements should be after the control that revealed them.

Or maybe just after the "Hostname" field.

[Mte90]

fieldset with class and hidden with js working

[Mte90]

moved connection type after the hostname

[Mte90]

I've created two patch, the first have the same effect of the old table system. The fieldset that contain the ssh part have a class and is hidden on start if is not picked the ssh connection and js now work again.
The second move the connection type after the hostname, i think that is the better position and at the start of the form is better, the user fill the form following the fields order.

[Mte90]

There are any plan for the 4.6?

[Mte90]

As I can see the patch need a refresh for the 4.6 because the line numbers are changed but the code to edit it is the same.

[afercia]

The JS part needs to be reviewed a bit too :)

[Mte90]

compatible with 4.6, js removed from file.php and moved on updates.js

[Mte90]

• The radio button are at the top of the fields
• jQuery code in file.php moved to updates.js
• Added a check to hide the fieldset for the ssh2 fields

[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?

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

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 :)

[Mte90]

I will look for this improvements/suggestions in the next week :-)

[Mte90]

improvement after dev-feedback

[Mte90]

The version 3 contain:

• Js improvement to simplify the code that works in both (modal/page) and with less events
• Moved the connection box as examplained
• Moved also the label "Enter the location on the server where the public ..."

[afercia]

Patch looks good. In 34376.4.patch​ added some JavaScript and CSS tweaking. Screenshots with patch applied:

[afercia]

In 37467:

Refine the FTP credentials form interaction.

Properly toggle SSH2 Authentication Keys fieldset visibility.
JavaScript and CSS clean-up.

Props Mte90.
Fixes #34376.