Dashboard v2: Add the ability to configure sftp and ssh #103783

arthur791004 · 2025-05-28T16:39:09Z

Proposed Changes

This PR implements the ability to configure sftp and ssh
Out of scope
- ~~Use InputControlSuffixWrapper to add "copy" icon~~
- ReauthRequired - We need this component before requesting ssh keys on v1. See DOTCOM-13441
- ~~Use DataForm - Do we need to use it for the readonly form?~~ Done by by 3726113 and ceeef5b
- Styles of the Callout component - The title styles doesn't match the design. See DOTCOM-13434

Site Settings

SSH is not available	SSH is available	Both enabled

Callout

The current styles of title is weird...

Create credentials (Need to confirm the UI)

SFTP Enabled

SSH Enabled

SSH Key Attached

Why are these changes being made?

Testing Instructions

Go to /v2
Pick any site
Go to settings
Configure your sftp & ssh settings
Ensure everything works as expected

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
- For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

github-actions · 2025-05-28T16:39:23Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-146626

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-146626&env=jetpack

Automattic for Agencies live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-146626&env=a8c-for-agencies

matticbot · 2025-05-28T16:48:52Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~903 bytes added 📈 [gzipped])

name                    parsed_size           gzip_size
entry-dashboard-dotcom      +5747 B  (+0.5%)     +903 B  (+0.3%)
entry-dashboard-a4a         +5747 B  (+0.5%)     +903 B  (+0.3%)
entry-subscriptions          +551 B  (+0.0%)     +198 B  (+0.0%)
entry-stepper                +551 B  (+0.0%)     +197 B  (+0.0%)
entry-reauth-required        +551 B  (+0.0%)     +198 B  (+0.0%)
entry-main                   +551 B  (+0.0%)     +199 B  (+0.0%)
entry-login                  +551 B  (+0.0%)     +198 B  (+0.0%)
entry-domains-landing        +551 B  (+0.1%)     +198 B  (+0.1%)
entry-browsehappy            +551 B  (+0.3%)     +198 B  (+0.3%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~1310 bytes added 📈 [gzipped])

name                parsed_size           gzip_size
site-settings           +2643 B  (+0.2%)     +592 B  (+0.1%)
hosting                 +2643 B  (+0.1%)     +688 B  (+0.1%)
reader                   +306 B  (+0.0%)      +44 B  (+0.0%)
patterns                 +306 B  (+0.0%)      +44 B  (+0.0%)
staging-site              +45 B  (+0.0%)      +14 B  (+0.0%)
sites-dashboard           +45 B  (+0.0%)      +14 B  (+0.0%)
site-performance          +45 B  (+0.0%)      +14 B  (+0.0%)
site-monitoring           +45 B  (+0.0%)      +14 B  (+0.0%)
site-logs                 +45 B  (+0.0%)      +14 B  (+0.0%)
plans                     +45 B  (+0.0%)      +14 B  (+0.0%)
overview                  +45 B  (+0.0%)      +14 B  (+0.0%)
github-deployments        +45 B  (+0.0%)      +14 B  (+0.0%)
domains                   +45 B  (+0.0%)      +14 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~44 bytes added 📈 [gzipped])

name                                                                         parsed_size           gzip_size
async-load-comment-block-editor                                                   +306 B  (+0.0%)      +44 B  (+0.0%)
async-load-automattic-global-styles-src-components-global-styles-variations       +306 B  (+0.0%)      +44 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

arthur791004 · 2025-06-02T06:28:34Z

client/dashboard/sites/settings-sftp-ssh/clipboard-input-control.tsx

+import { copySmall } from '@wordpress/icons';
+import React, { useState, useEffect } from 'react';
+
+export default function ClipboardInputControl(


Do we need to move it to @automattic/dataviews or @automattic/components?

Maybe we should wait for a bit before we propose lifting it up into a package. See if it changes much. The "Copied" message for example seems like it might be hard to standardise? The DS might not know the best way for your app to show success messages?

arthur791004 · 2025-06-02T06:29:25Z

client/dashboard/sites/settings-sftp-ssh/sftp-card.tsx

+					</Text>
+				</VStack>
+				<VStack spacing={ 4 } style={ { padding: '8px 0' } }>
+					<ClipboardInputControl


Do we need to apply DataForm here 🤔

I know they are readonly, but I think we should. We want these controls to have a style consistent with the rest of the dashboard, and using DataForm seems like the correct way to do this. Even if it is over powered.

arthur791004 · 2025-06-02T06:30:52Z

client/dashboard/sites/settings-sftp-ssh/ssh-card.tsx

+						) }
+					</Text>
+				</VStack>
+				<VStack spacing={ 4 } style={ { padding: '8px 0' } }>


There are 3 actions here

toggle the ssh access

attach ssh key

detach ssh key

Do we need to apply DataForm here 🤔

Personally I think it makes sense to use a DataForm here too. Just IMO. It's the best way to keep all the forms looking and working the same. E.g. errors can be returned by these 3 actions, and we want to the error messages to be implemented the same way.

Perhaps the attach button should be the one with the type="submit" since it's the one that actually makes use of one of the form fields?

fushar

On Pro plan, SSH is not available; should we adjust the title similar to v1 where it says only SFTP 🥲

arthur791004 · 2025-06-02T07:18:41Z

On Pro plan, SSH is not available; should we adjust the title similar to v1 where it says only SFTP 🥲

Hmmm...but how about the Upsell? I think the callout needs to mention both sftp and ssh. Also, how can people know they need to upgrade to have ssh from the Pro plan (is it still available?)? It seems we still need another upsell if the ssh is not available 🤔

arthur791004 · 2025-06-02T07:31:25Z

Good points.... I think we can create an issue to solve later.

Created. See DOTCOM-13410.

arthur791004 · 2025-06-02T07:40:59Z

client/dashboard/sites/settings-sftp-ssh/clipboard-input-control.tsx

+					<Button
+						size="small"
+						icon={ copySmall }
+						label={


It doesn't seem to have a way to display the tooltip immediately after the click...

Could we show a snackbar notification? It's different from how Calypso does it, but seems to fit in with the design system.

I don't think changing the button's label to show the "Copied" message is correct for a11y.

Yes, changing the aria-label like this is good for accessibility, as long as you manage the timing and state properly. It gives meaningful, real-time feedback to users relying on assistive technology.

I asked ChatGPT, and it seems fine 🙂 I also noticed that we’ve done the same thing in several other places so it should be okay.

Show the snackbar by 160b813

jasmussen · 2025-06-02T08:40:02Z

Nice one, gave this a quick shot:

Looks like you fixed the heading size? Looks more or less right to me.

Separately (i.e. not for this PR), the button to link to "upgrade plan", instead of putting a premium plan in your cart and taking you to checkout, can we link to a plans page? I'm imagining something like https://wordpress.com/plans/, but without a need to select a site because you've already drilled into a particular site when you click this button, so we know which one to pick for you. CC: @matt-west

Motivation: as a user I'd like to pore over which features I get and how I can get the best deal. Just going to the checkout cart I'm not sure what I'm getting.

jasmussen · 2025-06-02T08:50:03Z

Oh, curious note. If you navigate from settings overview to Settings → SFTP, the heading font size is correct.

But if when you're on that page, you reload, the heading font size is small:

The font size is that of an H6. Some issues with that, firstly that this should not be an H6, because the only other heading on that page says "SFTP/SSH", and is an H1. If we need for it to be a heading, it should be an H2, the next in line from H1.

Because this is using the "Callout" reusable component (kudos), this to me says we should not use a heading at all for that element. It can be a paragraph of bold text. That way it can work in any context without worrying about the accessibility of a heading hierarchy. On the flipside, if we DO want that to be a heading, then we need to either have the heading level smartly set by the system to always be 1 lower than the preceeding heading, OR have a prop so the implementer can choose the heading level.

Finally there's the font size itself. It is designed to be a "LG Heading" (i.e. 15px and medium font weight). Whether a paragraph or a heading, that element should always have that visual style, and it should not be subject to an otherwise type hierarchy. This is both a comment on this CSS:

I don't think any of the headings should ever be smaller than the 13px font size. If this needs a separate issue, worth opening one at the DS level.

Separately, it could be an argument for having heading level CSS classes. WordPress.org does this in a reflection that the semantic heading hierarchy doesn't always match the visual hierarchy. For example theme card headings are H2s and have these CSS classes:

has-normal-font-size has-inter-font-family

Finally, there's a double border at the bottom of all summary groups:

In fact this appears to be coming from an empty list item in every one of those groups:

arthur791004 · 2025-06-02T15:04:28Z

Because this is using the "Callout" reusable component (kudos), this to me says we should not use a heading at all for that element. It can be a paragraph of bold text. That way it can work in any context without worrying about the accessibility of a heading hierarchy. On the flipside, if we DO want that to be a heading, then we need to either have the heading level smartly set by the system to always be 1 lower than the preceeding heading, OR have a prop so the implementer can choose the heading level.

cc @p-jackson What do you think 🤔

p-jackson · 2025-06-03T02:29:10Z

Finally, there's a double border at the bottom of all summary groups:

Looks like that is going to be fixed by #103876

Because this is using the "Callout" reusable component (kudos), this to me says we should not use a heading at all for that element. It can be a paragraph of bold text. That way it can work in any context without worrying about the accessibility of a heading hierarchy. On the flipside, if we DO want that to be a heading, then we need to either have the heading level smartly set by the system to always be 1 lower than the preceeding heading, OR have a prop so the implementer can choose the heading level.

Fwiw the callout component does have a headingLevel prop for setting the appropriate semantic level. However taking a look at how callouts are used in the site settings screens, I realise that callout titles probably shouldn't be forced into being a semantic heading. Defaulting to bold text does make more sense.

I implemented Callout for examples like the one below, where the page content was inert, and the callout title really is the first accessible heading below the nav bar.

However the site settings are different. The page content can't be inert because the user needs access to the breadcrumb. In this case the callout title shouldn't be a heading, the page already has an <h1>

We did discuss the correct way to implement the title prop in the original Callout PR. I was in favour of forcing it to be a heading, but seems that it should really be possible to have it either as a heading or as bold text.

p-jackson · 2025-06-03T02:38:05Z

[Callouts] should really be possible to have it either as a heading or as bold text.

Created a follow up: DOTCOM-13434

p-jackson · 2025-06-03T02:56:06Z

client/dashboard/sites/settings-sftp-ssh/index.tsx

+		return null;
+	}
+
+	if ( ! canUseSftp( site ) ) {


This logic doesn't match the callout copy since simple sites with a Business Plan also match ! canUseSftp().

I shared thoughts in this discussion about why I don't think we should reuse the callout pattern in this situation QOBjujZah0UlGDDdLKZhzi-fi-563_32922#1245696310.

I don't think we have any UI for initiating atomic transfer yet? I personally think it'd be ok for this PR if it rendered some TODO text saying you need to transfer to atomic.

Oh, I see now that all of our atomic settings share this same issue 😃. I suppose it can be ignored for this PR then.

Yup exactly. I already created an issue for this 🙈 DOTCOM-13376

p-jackson · 2025-06-03T04:03:26Z

client/dashboard/sites/settings-sftp-ssh/clipboard-input-control.tsx

+					<Button
+						size="small"
+						icon={ copySmall }
+						label={


Could we show a snackbar notification? It's different from how Calypso does it, but seems to fit in with the design system.

I don't think changing the button's label to show the "Copied" message is correct for a11y.

p-jackson · 2025-06-03T04:05:30Z

client/dashboard/sites/settings-sftp-ssh/clipboard-input-control.tsx

+import { copySmall } from '@wordpress/icons';
+import React, { useState, useEffect } from 'react';
+
+export default function ClipboardInputControl(


Maybe we should wait for a bit before we propose lifting it up into a package. See if it changes much. The "Copied" message for example seems like it might be hard to standardise? The DS might not know the best way for your app to show success messages?

p-jackson · 2025-06-03T04:17:03Z

client/dashboard/app/queries.ts

@@ -177,6 +190,18 @@ export function profileMutation() {
 	};
 }

+export function profileSshKeysQuery() {
+	return {
+		queryKey: [ 'profile', 'ssh-keys' ],


Should this query key have two parts? Or should it just be [ 'profile-ssh-keys' ]. I'm not sure, just trying to think it through. By giving it two parts it means that if you ever cleared the user profile data with queryClient.invalidateQueries( { queryKey: [ 'profile' ] } ); then it would also clear out this ssh key data too. Does that seem right? Maybe it is 🤷‍♂️

Yes, that's correct. However, I assume that when the profile data changes, the profile-related data is expected to be invalidated. Does that make sense?

See https://tanstack.com/query/latest/docs/framework/react/guides/query-invalidation#query-matching-with-invalidatequeries.

Perhaps it should be 🤔

p-jackson · 2025-06-03T04:23:37Z

client/dashboard/app/router.tsx

+		if ( canUseSftp( site ) ) {
+			await queryClient.ensureQueryData( siteSftpUsersQuery( siteSlug ) );
+		}
+
+		if ( canUseSsh( site ) ) {
+			await queryClient.ensureQueryData( siteSshAccessStatusQuery( siteSlug ) );
+		}


Promise.all() doesn't mind if you pass it a boolean (it's as if its passed an already resolved boolean promise), so you can still run the queries in parallel like this.

Suggested change

if ( canUseSftp( site ) ) {

await queryClient.ensureQueryData( siteSftpUsersQuery( siteSlug ) );

}

if ( canUseSsh( site ) ) {

await queryClient.ensureQueryData( siteSshAccessStatusQuery( siteSlug ) );

}

return Promise.all( [

canUseSftp( site ) && queryClient.ensureQueryData( siteSftpUsersQuery( siteSlug ) ),

canUseSsh( site ) && queryClient.ensureQueryData( siteSshAccessStatusQuery( siteSlug ) ),

] );

Done by 276ce4c.

p-jackson · 2025-06-03T04:27:15Z

client/dashboard/sites/settings-sftp-ssh/sftp-card.tsx

+					</Text>
+				</VStack>
+				<VStack spacing={ 4 } style={ { padding: '8px 0' } }>
+					<ClipboardInputControl


I know they are readonly, but I think we should. We want these controls to have a style consistent with the rest of the dashboard, and using DataForm seems like the correct way to do this. Even if it is over powered.

p-jackson · 2025-06-03T04:38:05Z

client/dashboard/sites/settings-sftp-ssh/ssh-card.tsx

+						) }
+					</Text>
+				</VStack>
+				<VStack spacing={ 4 } style={ { padding: '8px 0' } }>


Personally I think it makes sense to use a DataForm here too. Just IMO. It's the best way to keep all the forms looking and working the same. E.g. errors can be returned by these 3 actions, and we want to the error messages to be implemented the same way.

Perhaps the attach button should be the one with the type="submit" since it's the one that actually makes use of one of the form fields?

fushar · 2025-06-03T04:47:17Z

client/dashboard/sites/settings-sftp-ssh/sftp-card.tsx

+						<Button
+							variant="secondary"
+							isBusy={ mutation.isPending }
+							onClick={ handleCreatePassword }
+						>
+							{ __( 'Reset password' ) }
+						</Button>


Maybe it's just me but I think this should have isDestructive because it's resetting your existing password without any confirmation

I think it’s fine because it’s not a scary button, and nothing will be broken or removed. People can feel free to reset their password if they forget it.

Resetting passwords can be a big deal though. The fact we only support a single FTP password is a limitation. The user might have saved the password into a GitHub Action environment variable. Deleting the password will break their CI pipeline.

+1. We should make it scary in order for users to take a step back and think hard what resetting the password would entail.

Many ways to scarify, by the way! dhW7zYBamXJIeJIE3wy5f4-fi-3074_108567

We have some guidelines around usage of isDestructive in the design system docs and it's mostly reserved for big actions like deleting an entire site. I don't think it's necessary here.

If we’re concerned that people will reset their password and break other apps that are relying on that password we should present a confirmation dialog instead.

I see, thanks! Let's not change anything I think for now 😄

fushar · 2025-06-03T07:03:13Z

client/dashboard/sites/settings-sftp-ssh/ssh-card.tsx

+						icon={ trash }
+						label={ __( 'Detach' ) }
+						isBusy={ isBusy }
+						style={ { margin: '-6px' } }


Would it make more sense to simply reduce the HStack's spacing?

I don't think so. The reason for the negative margin is because the button has padding to increase the selection area (I’m guessing?), but that caused it to be misaligned with the layout. Maybe the margin should be applied inside the button instead, if that’s the reason 🤔

fushar · 2025-06-03T07:04:52Z

client/dashboard/sites/settings-sftp-ssh/summary.tsx

+	return (
+		<RouterLinkSummaryButton
+			to={ `/sites/${ site.slug }/settings/sftp-ssh` }
+			title="SFTP/SSH"


We're translating the settings title but not here. Let's be consistent :)

Updated by ee9d5cd.

fushar · 2025-06-03T07:05:30Z

client/dashboard/sites/settings-sftp-ssh/summary.tsx

+		<RouterLinkSummaryButton
+			to={ `/sites/${ site.slug }/settings/sftp-ssh` }
+			title="SFTP/SSH"
+			density="medium"


We now need to pass density from the parent SummaryButtonList; please check the other setting summaries on trunk.

Updated by 809b42c.

fushar · 2025-06-03T07:35:11Z

client/dashboard/sites/settings-sftp-ssh/ssh-card.tsx

+	return (
+		<Card>
+			<CardBody>
+				<VStack style={ { paddingBottom: '12px' } }>


Seems like we should standardize card's heading and description into a component, otherwise we will keep inventing custom padding, margin, text sizes, ... 😄 (just rant, not really a comment)

Totally agreed 😭

arthur791004 · 2025-06-03T08:09:01Z

Personally I think it makes sense to use a DataForm here too. Just IMO. It's the best way to keep all the forms looking and working the same. E.g. errors can be returned by these 3 actions, and we want to the error messages to be implemented the same way.

Perhaps the attach button should be the one with the type="submit" since it's the one that actually makes use of one of the form fields?

I changed to use DataForm by 3726113 and ceeef5b. However, I didn’t set the button type to submit, since both forms have optional buttons, and it feels a bit odd to trigger a submit behavior when the buttons might not even be present 🤔

matticbot · 2025-06-03T08:12:30Z

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

help-center
notifications
wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug feat/dashboard-v2-ssh on your sandbox.

jasmussen · 2025-06-03T08:14:31Z

I was in favour of forcing it to be a heading, but seems that it should really be possible to have it either as a heading or as bold text.

Thanks for the context! I've no strong opinion on whether it's a heading or bold text, just noting that if it is a heading, we need CSS that sets the visual appearance to always be the same regardless of what styles (notably font sizes and weight) might be inherited from the default h1-6 definitions.

matt-west · 2025-06-03T09:56:56Z

Separately (i.e. not for this PR), the button to link to "upgrade plan", instead of putting a premium plan in your cart and taking you to checkout, can we link to a plans page? I'm imagining something like https://wordpress.com/plans/, but without a need to select a site because you've already drilled into a particular site when you click this button, so we know which one to pick for you. CC: @matt-west

Agreed, although perhaps this is something we should test in the future. I think we already have the ability to show a condensed version of the plans grid that only shows a sub-set of plans.

fushar · 2025-06-03T10:50:07Z

client/dashboard/sites/settings-sftp-ssh/clipboard-input-control.tsx

+import { copySmall } from '@wordpress/icons';
+import React, { useState, useEffect } from 'react';
+
+export default function ClipboardInputControl( {


Can we move this to client/dashboard/components? I will need it for the Coming Soon's "share site" feature 🙈

fushar · 2025-06-03T10:51:57Z

client/dashboard/sites/settings-sftp-ssh/index.tsx

+
+	return (
+		<PageLayout size="small" header={ <SettingsPageHeader title={ __( 'SFTP/SSH' ) } /> }>
+			<VStack spacing={ 8 }>


I believe PageLayout already has <VStack spacing={8}> for the children, can we check 🤔

Oh you're right!

fushar · 2025-06-03T10:56:57Z

client/dashboard/sites/settings-sftp-ssh/ssh-card.tsx

+						label={ field.label }
+						value={ value }
+						options={ field.elements ?? [] }
+						onChange={ ( value ) => onChange( { [ id ]: value } ) }


I don't think onChange is defined here?

I think we can just use the newly created

return <RequiredSelect {...props} />;

matt-west

Thanks for working on this @arthur791004. I left some comments.

I’m aware that DOTCOM-13376 also impacts the SFTP/SSH section. I’ll follow-up with a design for that ASAP.

matt-west · 2025-06-03T11:00:50Z

client/dashboard/sites/settings-sftp-ssh/clipboard-input-control.tsx

+								: sprintf(
+										/* translators: %s is the field to copy */
+										__( 'Copy %s' ),
+										props.label


Is it possible for use to downcast the field name?

At the moment we show Copy Username, but to be consistent with our copy guidelines this should really be sentence case, so Copy username. We’d need to watch out for cases where the field name is an acronym though, like Copy URL which should remain all-caps.

The same applies to the Snackbar we show when the value is copied.

This should really be:

Copied connection command to clipboard.

Let me think about how to do it.

Maybe for this first iteration, if it's too convoluted, we can just show Copied!?

(Trying to unblock things here)

Copied! is a bit too generic for the snackbar. If you copy a few different fields in quick successions, you'll end up with a bunch of identical snackbars.

matt-west · 2025-06-03T11:04:20Z

client/dashboard/sites/settings-sftp-ssh/enable-sftp-card.tsx

+	return (
+		<Card>
+			<CardBody>
+				<Text variant="muted" lineHeight="20px" style={ { paddingBottom: '12px' } } as="p">


Let’s add a Get started with SFTP/SSH heading here. It helps resolve an issue where's there's not enough top padding on the card.

matt-west · 2025-06-03T11:05:07Z

client/dashboard/sites/settings-sftp-ssh/enable-sftp-card.tsx

+			onError: () => {
+				createErrorNotice(
+					__(
+						'Sorry, we had a problem retrieving your sftp user details. Please refresh the page and try again.'


SFTP should be all-caps as it's an acronym.

client/dashboard/sites/settings-sftp-ssh/index.tsx

matt-west · 2025-06-03T11:08:47Z

client/dashboard/sites/settings-sftp-ssh/sftp-card.tsx

+		createSuccessNotice(
+			sprintf(
+				/* translators: %s is the copied field */
+				__( 'Copied %s to clipboard.' ),


This should be sentence case if possible. At the moment it can appear as Copied Connection command to clipboard.

matt-west · 2025-06-03T11:16:20Z

client/dashboard/sites/settings-sftp-ssh/ssh-card.tsx

+		createSuccessNotice(
+			sprintf(
+				/* translators: %s is the copied field */
+				__( 'Copied %s to clipboard.' ),


Ditto comment about sentence casing the string so this becomes:

Copied connection command to clipboard.

matt-west · 2025-06-03T11:17:59Z

client/dashboard/sites/settings-sftp-ssh/ssh-card.tsx

+		);
+	};
+
+	const handleToggleSshAccess = () => {


We should show a Snackbar when SSH is enabled/disabled.

matt-west · 2025-06-03T11:19:49Z

client/dashboard/sites/settings-sftp-ssh/ssh-card.tsx

+						/>
+					) }
+				</VStack>
+				{ showSshKeysSelect && (


There's no way to add an SSH key if you don’t already have one attached to your account.

We should disable the key select and attach button in this case, but maintain the button for users to add a new SSH key.

Nice catch!

matt-west · 2025-06-03T11:28:02Z

client/dashboard/sites/settings-sftp-ssh/ssh-card.tsx

+	onDetach: ( siteSshKey: SiteSshKey ) => void;
+} ) => {
+	return (
+		<Card>


The padding and border-radius are off on this card.

We might be able to fix the padding issue by setting the size prop to small.

The border-radius is trickier as the component only appears to support a single isRounded prop. We typically reduce the border-radius of elements that are nested within a card. In this case we should use radius-s (2px) so the SSH key card matches the radius used on form inputs. If the only way to achieve this is a hacky CSS override, then perhaps we skip it. It would be good for the Card component to support this flexibility though.

I think the CSS is the only way 🥲

Okay, let's leave the radius as-is then. I'd rather not introduce tech debt.

matt-west · 2025-06-03T11:30:21Z

client/dashboard/sites/settings-sftp-ssh/summary.tsx

+
+	const badges = [
+		{
+			text: sftpEnabled ? __( 'SFTP enabled' ) : __( 'SFTP disabled' ),


I don't think badges for disabled states are strictly needed in the settings list. They can add a lot of unnecessary clutter to the settings list.

This feels like something we should agree on at the design system-level though. @jasmussen, @jameskoster, do you have an opinion on whether to display badges for disabled states in SummaryButtons?

arthur791004 force-pushed the feat/dashboard-v2-ssh branch 3 times, most recently from 372107a to 832bb2c Compare May 29, 2025 16:01

arthur791004 changed the title ~~WIP Dashboard v2: Add the ability to configure sftp and ssh~~ Dashboard v2: Add the ability to configure sftp and ssh May 29, 2025

arthur791004 self-assigned this May 29, 2025

arthur791004 force-pushed the feat/dashboard-v2-ssh branch from 832bb2c to a328796 Compare June 2, 2025 06:13

arthur791004 marked this pull request as ready for review June 2, 2025 06:23

arthur791004 requested review from youknowriad and a team as code owners June 2, 2025 06:23

matticbot added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jun 2, 2025

arthur791004 commented Jun 2, 2025

View reviewed changes

fushar reviewed Jun 2, 2025

View reviewed changes

arthur791004 added 11 commits June 2, 2025 15:18

Dashboard v2: Add sftp/ssh data and queries

6f7acb9

Dashboard v2: Add EnableSftpCard

f54fa75

Dashboard v2: Add SftpCard

c3d53a3

Dashboard v2: Add SshCard

f1f1fd4

Dashboard v2: Display sftp callout

10c83a6

Make it work

2919e36

Dashboard v2: Implement ClipboardInputControl

71a43ef

Add label to trash icon and use isBusy instead of disabled

42a166d

Fix types

e142679

Update EnableSftpCard stypes

4888a3f

Update SftpCard stypes

d588c3a

arthur791004 requested review from matt-west and jasmussen June 2, 2025 07:39

arthur791004 commented Jun 2, 2025

View reviewed changes

p-jackson reviewed Jun 3, 2025

View reviewed changes

fushar reviewed Jun 3, 2025

View reviewed changes

Address feedback

276ce4c

fushar reviewed Jun 3, 2025

View reviewed changes

Show snackbar when the value is copied

160b813

fushar reviewed Jun 3, 2025

View reviewed changes

arthur791004 added 2 commits June 3, 2025 15:08

Update density of the summary button

809b42c

Display error notices

86beba3

fushar reviewed Jun 3, 2025

View reviewed changes

arthur791004 added 2 commits June 3, 2025 16:01

Use DataForm on SftpCard

3726113

Use DataForm on SshCard

ceeef5b

Translate SFTP/SSH

ee9d5cd

fushar reviewed Jun 3, 2025

View reviewed changes

matt-west reviewed Jun 3, 2025

View reviewed changes

Dashboard v2: Add the ability to configure sftp and ssh #103783

Are you sure you want to change the base?

Dashboard v2: Add the ability to configure sftp and ssh #103783

Uh oh!

Conversation

arthur791004 commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Site Settings

Callout

Create credentials (Need to confirm the UI)

SFTP Enabled

SSH Enabled

SSH Key Attached

Why are these changes being made?

Testing Instructions

Pre-merge Checklist

Uh oh!

github-actions bot commented May 28, 2025 • edited by matticbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matticbot commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fushar left a comment

Choose a reason for hiding this comment

Uh oh!

arthur791004 commented Jun 2, 2025

Uh oh!

arthur791004 commented Jun 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasmussen commented Jun 2, 2025

Uh oh!

jasmussen commented Jun 2, 2025

Uh oh!

arthur791004 commented Jun 2, 2025

Uh oh!

p-jackson commented Jun 3, 2025

Uh oh!

p-jackson commented Jun 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

arthur791004 commented May 28, 2025 •

edited

Loading

github-actions bot commented May 28, 2025 •

edited by matticbot

Loading

matticbot commented May 28, 2025 •

edited

Loading

jasmussen Jun 3, 2025 •

edited

Loading