Skip to content
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

Add Secrets view and deploy with Secrets to extension #2288

Merged
merged 19 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export type DeployMsg = AnyWebviewToHostMessage<
credentialName: string;
configurationName: string;
projectDir: string;
secrets?: Record<string, string>;
}
>;

Expand Down
12 changes: 3 additions & 9 deletions extensions/vscode/src/views/homeView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable {
credentialName: string,
configurationName: string,
projectDir: string,
secrets?: Record<string, string>,
) {
try {
const api = await useApi();
Expand All @@ -256,6 +257,7 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable {
credentialName,
configurationName,
projectDir,
secrets,
);
deployProject(response.data.localId, this.stream);
} catch (error: unknown) {
Expand All @@ -270,6 +272,7 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable {
msg.content.credentialName,
msg.content.configurationName,
msg.content.projectDir,
msg.content.secrets,
);
}

Expand Down Expand Up @@ -1398,15 +1401,6 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable {
return await this.sendRefreshedFilesLists();
}, DebounceDelaysMS.file);

public initiatePublish(target: PublishProcessParams) {
this.initiateDeployment(
target.deploymentName,
target.credentialName,
target.configurationName,
target.projectDir,
);
}

sagerb marked this conversation as resolved.
Show resolved Hide resolved
public async handleFileInitiatedDeploymentSelection(uri: Uri) {
// Guide the user to create a new Deployment with that file as the entrypoint
// if one doesn’t exist
Expand Down
2 changes: 2 additions & 0 deletions extensions/vscode/webviews/homeView/src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<EvenEasierDeploy class="easy-deploy-container" />
<template v-if="home.selectedConfiguration">
<ProjectFiles v-model:expanded="projectFilesExpanded" />
<Secrets />
<PythonPackages />
<RPackages />
<Credentials />
Expand All @@ -21,6 +22,7 @@ import { ref } from "vue";
import OverlayableView from "src/components/OverlayableView.vue";
import EvenEasierDeploy from "src/components/EvenEasierDeploy.vue";
import ProjectFiles from "src/components/views/projectFiles/ProjectFiles.vue";
import Secrets from "src/components/views/secrets/Secrets.vue";
import PythonPackages from "src/components/views/PythonPackages.vue";
import RPackages from "src/components/views/RPackages.vue";
import Credentials from "src/components/views/Credentials.vue";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ const onPublishStartMsg = () => {
};
const onPublishFinishSuccessMsg = () => {
const home = useHomeStore();
home.clearSecretValues();
home.publishInProgress = false;
home.lastContentRecordResult = `Last Deployment was Successful`;
home.lastContentRecordMsg = "";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,22 @@ const deploy = () => {
return;
}

// Only send up secrets that have values
const secrets: Record<string, string> = {};
home.secrets.forEach((value, name) => {
if (value) {
secrets[name] = value;
}
});

hostConduit.sendMsg({
kind: WebviewToHostMessageType.DEPLOY,
content: {
deploymentName: home.selectedContentRecord.saveName,
configurationName: home.selectedConfiguration.configurationName,
credentialName: home.serverCredential.name,
projectDir: home.selectedContentRecord.projectDir,
secrets: secrets,
},
});
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<template>
<input
v-model="model"
ref="input"
class="sidebar-input"
:aria-label="ariaLabel"
@keydown.prevent.enter="$emit('submit')"
@keydown.escape="$emit('cancel')"
/>
</template>

<script setup lang="ts">
import { computed, ref } from "vue";
const model = defineModel();

const props = defineProps<{
label: string;
}>();

defineEmits(["submit", "cancel"]);

const input = ref<HTMLInputElement | null>(null);

const ariaLabel = computed(
() => `${props.label}. Press Enter to confirm or Escape to cancel.`,
);

const select = () => {
input.value?.select();
};

defineExpose({ select });
</script>

<style lang="scss" scoped>
.sidebar-input {
background-color: var(--vscode-input-background);
border: 1px solid var(--vscode-input-border, transparent);
color: var(--vscode-input-foreground);
line-height: 20px;
padding: 0;
outline-color: var(--vscode-focusBorder);
outline-offset: -1px;
outline-style: solid;
outline-width: 1px;
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@
/>
<div class="tree-item-label-container">
<span class="tree-item-title" data-automation="req">{{ title }}</span>
<span v-if="description" class="tree-item-description">
<span v-if="$slots.description" class="tree-item-description">
<slot name="description" />
</span>
<span v-else-if="description" class="tree-item-description">
{{ description }}
</span>
</div>
Expand Down Expand Up @@ -79,6 +82,7 @@ withDefaults(defineProps<Props>(), {

defineSlots<{
default(props: { indentLevel: number }): any;
description(): any;
postDecor(): any;
}>();

Expand Down Expand Up @@ -151,6 +155,7 @@ const toggleExpanded = () => {

.tree-item-label-container {
flex: 1;
display: flex;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to bring in flex for the label container so that the input in the description span could be displayed the way we want. This required bringing down the

min-width: 0;
overflow: hidden;
text-overflow: ellipsis;

CSS for dealing with overflowing with ellipsis.

min-width: 0;
overflow: hidden;
text-overflow: ellipsis;
Expand All @@ -159,13 +164,24 @@ const toggleExpanded = () => {
line-height: 22px;
color: inherit;
white-space: pre;
min-width: 0;
overflow: hidden;
text-overflow: ellipsis;
}

.tree-item-description {
line-height: 22px;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line-height has returned. Now that we are using display: flex here the span is no longer being rendered the same way. Without this the height changes, and the description wasn't on the baseline.

font-size: 0.9em;
margin-left: 0.5em;
opacity: 0.7;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since description can contain an input now we don't want the description to always be opacity: 0.7. Now we apply this only if the description doesn't contain an input (see below)

flex-shrink: 100000;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This retains the behavior we had before where the description would shrink before the title.

white-space: pre;
min-width: 0;
overflow: hidden;
text-overflow: ellipsis;

&:not(:has(input)) {
opacity: 0.7;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
<template>
<TreeItem
:title="name"
:actions="actions"
codicon="codicon-lock-small"
:tooltip="tooltip"
align-icon-with-twisty
>
<template #description>
<SidebarInput
v-if="isEditing"
ref="input"
v-model="inputValue"
class="w-full"
label="Type secret value"
@submit="updateSecret"
@cancel="isEditing = false"
/>
<template v-else-if="secretValue">••••••</template>
</template>
</TreeItem>
</template>

<script setup lang="ts">
import { computed, ref, nextTick } from "vue";

import TreeItem from "src/components/tree/TreeItem.vue";
import SidebarInput from "src/components/SidebarInput.vue";
import { useHomeStore } from "src/stores/home";
import { ActionButton } from "src/components/ActionToolbar.vue";

interface Props {
name: string;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have expected that this component would be a UI component that would accept its name and value through a prop and emit a change when the value changed. I would then have expected the view using this component to have to deal with the storage of the secret value.

While this approach will work, it makes this component extremely dedicated to this particular input vs. having a component that allows its value to be edited. I think we would benefit from having it be a bit more general purpose, especially when you think of using it for environment variables as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually started writing it this way at the start, but found it more difficult to keep the home.secrets Map in line with the value that was being v-modeled in Secret.vue instead I made it just reflect the state of home.secrets.get('name') for the name key provided

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main difficulty when using v-model was grabbing the most recent value when swapping over to the input mode. It worked great when setting the value, but not when the value changed out from under the component.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you get a chance to play with the approach we discussed via zoom? That would be where outside component would pass in the map value from the store and then update the store when receiving the change emitted event from the component?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did mess around with this, and it ended up re-creating the Map in the Secrets component a bit as an array so each v-model accessed an index in that array. It felt duplicative to me.

}

const props = defineProps<Props>();

const input = ref<InstanceType<typeof SidebarInput> | null>(null);
const isEditing = ref(false);
const inputValue = ref<string>();

const home = useHomeStore();

const secretValue = computed(() => home.secrets.get(props.name));

const inputSecret = () => {
// Update inputValue in case the secret value has changed or been cleared
inputValue.value = secretValue.value;
isEditing.value = true;
// Wait for next tick to ensure the input is rendered
nextTick(() => input.value?.select());
};

const updateSecret = () => {
home.secrets.set(props.name, inputValue.value);
isEditing.value = false;
};

const tooltip = computed(() => {
if (secretValue.value) {
return "On the next deploy the new value will be set for the deployment.";
}

return "No value has been set. The value will not change on the next deploy.";
});

const actions = computed<ActionButton[]>(() => {
// Show no actions while the value is being edited
if (isEditing.value) {
return [];
}

const result = [
{
label: "Edit Value",
codicon: "codicon-pencil",
fn: inputSecret,
},
];

if (secretValue.value) {
result.push({
label: "Clear Value",
codicon: "codicon-x",
dotNomad marked this conversation as resolved.
Show resolved Hide resolved
fn: () => {
home.secrets.set(props.name, undefined);
},
});
}

return result;
});
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<template>
<TreeSection
title="Secrets"
:actions="sectionActions"
:count="home.secretsWithValueCount"
>
<template v-if="home.secrets.size">
<Secret v-for="[name] in home.secrets" :name="name" :key="name" />
</template>
<WelcomeView v-else>
<p>No secrets have been added to the configuration.</p>
</WelcomeView>
</TreeSection>
</template>

<script setup lang="ts">
import { computed } from "vue";

import { ActionButton } from "src/components/ActionToolbar.vue";
import TreeSection from "src/components/tree/TreeSection.vue";
import WelcomeView from "src/components/WelcomeView.vue";
import { useHomeStore } from "src/stores/home";
import Secret from "src/components/views/secrets/Secret.vue";

const home = useHomeStore();

const sectionActions = computed<ActionButton[]>(() => [
{
label: "Clear Values for all Secrets",
codicon: "codicon-clear-all",
fn: () => {
home.secrets.forEach((_, key) => {
home.secrets.set(key, undefined);
});
},
},
Comment on lines +28 to +36
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could optionally show this only if 1 or more secrets have values?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more I use this the more I want to do this, since the button moves if the count badge is showing or not.

]);
</script>
Loading