Skip to content

Commit

Permalink
UX enhancements for Attach ACR to Cluster (Azure#748)
Browse files Browse the repository at this point in the history
* rename 'connect' to 'attach'

* Avoid displaying fields that are not loaded

* add blurb

* make not-loaded fields disabled and add text to buttons

* Rename action and buttons

* make 'attach' button primary

* move inline action component to common folder

* use unabbreviated 'Azure Container Registry' in title
  • Loading branch information
peterbom authored Jun 19, 2024
1 parent 381e528 commit 7a77fe6
Show file tree
Hide file tree
Showing 25 changed files with 209 additions and 181 deletions.
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@
"title": "Kustomize Workflow (deprecated)"
},
{
"command": "aks.connectAcrToCluster",
"title": "Connect ACR to Cluster",
"command": "aks.attachAcrToCluster",
"title": "Attach ACR to Cluster",
"category": "AKS"
},
{
Expand Down Expand Up @@ -450,7 +450,7 @@
],
"aks.draftSubMenu": [
{
"command": "aks.connectAcrToCluster",
"command": "aks.attachAcrToCluster",
"group": "navigation"
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,27 @@ import { commands, window } from "vscode";
import { IActionContext } from "@microsoft/vscode-azext-utils";
import { getReadySessionProvider } from "../../auth/azureAuth";
import { failed, succeeded } from "../utils/errorable";
import { InitialSelection } from "../../webview-contract/webviewDefinitions/connectAcrToCluster";
import { ConnectAcrToClusterDataProvider, ConnectAcrToClusterPanel } from "../../panels/ConnectAcrToClusterPanel";
import { InitialSelection } from "../../webview-contract/webviewDefinitions/attachAcrToCluster";
import { AttachAcrToClusterDataProvider, AttachAcrToClusterPanel } from "../../panels/AttachAcrToClusterPanel";
import { getExtension } from "../utils/host";
import * as k8s from "vscode-kubernetes-tools-api";
import { getAksClusterTreeNode } from "../utils/clusters";

export type ConnectAcrToClusterParams = {
export type AttachAcrToClusterParams = {
initialSelection?: InitialSelection;
};

/**
* Allows the command to be invoked programmatically, in this case from the message handler
* of the 'Draft Workflow' webview.
*/
export function launchConnectAcrToClusterCommand(params: ConnectAcrToClusterParams) {
commands.executeCommand("aks.connectAcrToCluster", params);
export function launchAttachAcrToClusterCommand(params: AttachAcrToClusterParams) {
commands.executeCommand("aks.attachAcrToCluster", params);
}

export async function connectAcrToCluster(_context: IActionContext, target: unknown): Promise<void> {
export async function attachAcrToCluster(_context: IActionContext, target: unknown): Promise<void> {
const cloudExplorer = await k8s.extension.cloudExplorer.v1;
const params = getConnectAcrToClusterParams(cloudExplorer, target);
const params = getAttachAcrToClusterParams(cloudExplorer, target);

const extension = getExtension();
if (failed(extension)) {
Expand All @@ -39,15 +39,15 @@ export async function connectAcrToCluster(_context: IActionContext, target: unkn
// Explicitly pass empty initialSelection if not defined.
const initialSelection = params?.initialSelection || {};

const panel = new ConnectAcrToClusterPanel(extension.result.extensionUri);
const dataProvider = new ConnectAcrToClusterDataProvider(sessionProvider.result, initialSelection);
const panel = new AttachAcrToClusterPanel(extension.result.extensionUri);
const dataProvider = new AttachAcrToClusterDataProvider(sessionProvider.result, initialSelection);
panel.show(dataProvider);
}

function getConnectAcrToClusterParams(
function getAttachAcrToClusterParams(
cloudExplorer: k8s.API<k8s.CloudExplorerV1>,
params: unknown,
): ConnectAcrToClusterParams {
): AttachAcrToClusterParams {
const clusterNode = getAksClusterTreeNode(params, cloudExplorer);
if (succeeded(clusterNode)) {
return {
Expand All @@ -59,5 +59,5 @@ function getConnectAcrToClusterParams(
};
}

return params as ConnectAcrToClusterParams;
return params as AttachAcrToClusterParams;
}
4 changes: 2 additions & 2 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { Reporter, reporter } from "./commands/utils/reporter";
import installAzureServiceOperator from "./commands/azureServiceOperators/installAzureServiceOperator";
import { AzureResourceNodeContributor } from "./tree/azureResourceNodeContributor";
import { setAssetContext } from "./assets";
import { connectAcrToCluster } from "./commands/aksConnectAcrToCluster/connectAcrToCluster";
import { attachAcrToCluster } from "./commands/aksAttachAcrToCluster/attachAcrToCluster";
import {
configureStarterWorkflow,
configureHelmStarterWorkflow,
Expand Down Expand Up @@ -87,7 +87,7 @@ export async function activate(context: vscode.ExtensionContext) {
registerCommandWithTelemetry("aks.configureHelmStarterWorkflow", configureHelmStarterWorkflow);
registerCommandWithTelemetry("aks.configureKomposeStarterWorkflow", configureKomposeStarterWorkflow);
registerCommandWithTelemetry("aks.configureKustomizeStarterWorkflow", configureKustomizeStarterWorkflow);
registerCommandWithTelemetry("aks.connectAcrToCluster", connectAcrToCluster);
registerCommandWithTelemetry("aks.attachAcrToCluster", attachAcrToCluster);
registerCommandWithTelemetry("aks.draftDockerfile", draftDockerfile);
registerCommandWithTelemetry("aks.draftDeployment", draftDeployment);
registerCommandWithTelemetry("aks.draftWorkflow", draftWorkflow);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
ToVsCodeMsgDef,
ToWebViewMsgDef,
acrPullRoleDefinitionName,
} from "../webview-contract/webviewDefinitions/connectAcrToCluster";
} from "../webview-contract/webviewDefinitions/attachAcrToCluster";
import { Errorable, failed, getErrorMessage } from "../commands/utils/errorable";
import { SelectionType, getSubscriptions } from "../commands/utils/subscriptions";
import { acrResourceType, clusterResourceType, getResources } from "../commands/utils/azureResources";
Expand All @@ -27,9 +27,9 @@ import {
} from "../commands/utils/roleAssignments";
import { getManagedCluster } from "../commands/utils/clusters";

export class ConnectAcrToClusterPanel extends BasePanel<"connectAcrToCluster"> {
export class AttachAcrToClusterPanel extends BasePanel<"attachAcrToCluster"> {
constructor(extensionUri: Uri) {
super(extensionUri, "connectAcrToCluster", {
super(extensionUri, "attachAcrToCluster", {
// Reference data responses
getSubscriptionsResponse: null,
getAcrsResponse: null,
Expand All @@ -43,14 +43,14 @@ export class ConnectAcrToClusterPanel extends BasePanel<"connectAcrToCluster"> {
}
}

export class ConnectAcrToClusterDataProvider implements PanelDataProvider<"connectAcrToCluster"> {
export class AttachAcrToClusterDataProvider implements PanelDataProvider<"attachAcrToCluster"> {
constructor(
readonly sessionProvider: ReadyAzureSessionProvider,
readonly initialSelection: InitialSelection,
) {}

getTitle(): string {
return "Connect ACR to Cluster";
return "Attach ACR to Cluster";
}

getInitialState(): InitialState {
Expand All @@ -59,7 +59,7 @@ export class ConnectAcrToClusterDataProvider implements PanelDataProvider<"conne
};
}

getTelemetryDefinition(): TelemetryDefinition<"connectAcrToCluster"> {
getTelemetryDefinition(): TelemetryDefinition<"attachAcrToCluster"> {
return {
// Reference data requests
getSubscriptionsRequest: false,
Expand Down
18 changes: 9 additions & 9 deletions src/panels/draft/DraftWorkflowPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { BasePanel, PanelDataProvider } from "../BasePanel";
import {
ExistingFile,
InitialState,
LaunchConnectAcrToClusterParams,
LaunchAttachAcrToClusterParams,
PickFilesIdentifier,
ToVsCodeMsgDef,
ToWebViewMsgDef,
Expand Down Expand Up @@ -33,9 +33,9 @@ import { ReadyAzureSessionProvider } from "../../auth/types";
import { getResources } from "../../commands/utils/azureResources";
import { launchDraftCommand } from "./commandUtils";
import {
ConnectAcrToClusterParams,
launchConnectAcrToClusterCommand,
} from "../../commands/aksConnectAcrToCluster/connectAcrToCluster";
AttachAcrToClusterParams,
launchAttachAcrToClusterCommand,
} from "../../commands/aksAttachAcrToCluster/attachAcrToCluster";

export class DraftWorkflowPanel extends BasePanel<"draftWorkflow"> {
constructor(extensionUri: Uri) {
Expand Down Expand Up @@ -101,7 +101,7 @@ export class DraftWorkflowDataProvider implements PanelDataProvider<"draftWorkfl
openFileRequest: false,
launchDraftDockerfile: false,
launchDraftDeployment: false,
launchConnectAcrToCluster: false,
launchAttachAcrToCluster: false,
};
}

Expand All @@ -126,7 +126,7 @@ export class DraftWorkflowDataProvider implements PanelDataProvider<"draftWorkfl
launchDraftCommand("aks.draftDeployment", {
workspaceFolder: this.workspaceFolder,
}),
launchConnectAcrToCluster: (params) => this.handleLaunchConnectAcrToCluster(params),
launchAttachAcrToCluster: (params) => this.handleLaunchAttachAcrToCluster(params),
};
}

Expand Down Expand Up @@ -323,8 +323,8 @@ export class DraftWorkflowDataProvider implements PanelDataProvider<"draftWorkfl
window.showTextDocument(Uri.file(filePath));
}

private handleLaunchConnectAcrToCluster(params: LaunchConnectAcrToClusterParams) {
const commandParams: ConnectAcrToClusterParams = {
private handleLaunchAttachAcrToCluster(params: LaunchAttachAcrToClusterParams) {
const commandParams: AttachAcrToClusterParams = {
initialSelection: {
subscriptionId: params.initialSubscriptionId || undefined,
acrResourceGroup: params.initialAcrResourceGroup || undefined,
Expand All @@ -334,7 +334,7 @@ export class DraftWorkflowDataProvider implements PanelDataProvider<"draftWorkfl
},
};

launchConnectAcrToClusterCommand(commandParams);
launchAttachAcrToClusterCommand(commandParams);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,4 @@ export type ToWebViewMsgDef = {
};
};

export type ConnectAcrToClusterDefinition = WebviewDefinition<InitialState, ToVsCodeMsgDef, ToWebViewMsgDef>;
export type AttachAcrToClusterDefinition = WebviewDefinition<InitialState, ToVsCodeMsgDef, ToWebViewMsgDef>;
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ export type ToVsCodeMsgDef = {
openFileRequest: string;
launchDraftDockerfile: void;
launchDraftDeployment: void;
launchConnectAcrToCluster: LaunchConnectAcrToClusterParams;
launchAttachAcrToCluster: LaunchAttachAcrToClusterParams;
};

export type LaunchConnectAcrToClusterParams = {
export type LaunchAttachAcrToClusterParams = {
initialSubscriptionId: string | null;
initialAcrResourceGroup: string | null;
initialAcrName: string | null;
Expand Down
4 changes: 2 additions & 2 deletions src/webview-contract/webviewTypes.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Message, MessageContext, MessageDefinition, MessageHandler, MessageSink } from "./messaging";
import { ClusterPropertiesDefinition } from "./webviewDefinitions/clusterProperties";
import { ConnectAcrToClusterDefinition } from "./webviewDefinitions/connectAcrToCluster";
import { AttachAcrToClusterDefinition } from "./webviewDefinitions/attachAcrToCluster";
import { CreateClusterDefinition } from "./webviewDefinitions/createCluster";
import { DetectorDefinition } from "./webviewDefinitions/detector";
import { DraftDeploymentDefinition } from "./webviewDefinitions/draft/draftDeployment";
Expand Down Expand Up @@ -34,7 +34,7 @@ export type WebviewDefinition<
type AllWebviewDefinitions = {
style: TestStyleViewerDefinition;
clusterProperties: ClusterPropertiesDefinition;
connectAcrToCluster: ConnectAcrToClusterDefinition;
attachAcrToCluster: AttachAcrToClusterDefinition;
periscope: PeriscopeDefinition;
createCluster: CreateClusterDefinition;
detector: DetectorDefinition;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
row-gap: 0.5rem;
}

.inputContainer p {
margin-top: 0;
margin-bottom: 0.5rem;
}

.inputContainer .label {
grid-column: 1 / 2;
margin: 0.2rem 0;
Expand All @@ -13,7 +18,7 @@
.inputContainer .control {
grid-column: 2 / 3;
margin: 0.2rem 0;
width: 20rem;
width: 25rem;
}

.inputContainer .controlSupplement {
Expand All @@ -26,42 +31,9 @@
grid-column: 1 / 3;
}

.successIndicator {
color: var(--vscode-testing-iconPassed);
}

.errorIndicator {
color: var(--vscode-testing-iconFailed);
}

.actionItemList {
display: flex;
flex-direction: column;
align-items: stretch;
gap: 0.2rem;
}

.actionItem {
display: grid;
grid-template-columns: auto auto;
justify-content: stretch;
}

.actionItem .actionDescription {
grid-column: 1 / 2;
}

.actionItem .actionButtons {
grid-column: 2 / 3;
display: flex;
flex-direction: row;
justify-content: flex-end;
align-items: center;
gap: 0.5rem;
}

.inlineIcon {
vertical-align: middle;
margin-left: 0.3rem;
margin-right: 0.3rem;
}
Loading

0 comments on commit 7a77fe6

Please sign in to comment.