Save / Restore selection and expansion state on extension load #1373

sagerb · 2024-04-12T20:35:47Z

Intent

Resolves #1229
Resolves #1271

We will be using VSCode's storage to remember the last section and expansion states of the home view. These will be restored upon extension reload.

Type of Change

- Bug Fix
- New Feature
- Breaking Change
- Documentation
- Refactor
- Tooling

Approach

The tough part of this implementation was to find a clean way to restore the last selections to reflect the last state, vs the other times which we'll update the selections w/ business logic. So we needed to make sure that the business logic updates to selected configuration (which are made to reflect the last config used to deploy) and the update to credential (which is made to makes sure the account selected matches the deployment URL - either by validating the currently selected one or by switching to the first in the filtered list), are only executed when the deployment choice is changed by the user, rather than programmatically. By doing this, it allows us to set all three select box values to what was last persisted.

So we will restore the settings upon initialization of the Home View WebViewView, and then we record the settings into VSCode storage each time they are changed (either the Home View Deploy button is collapsed or expanded, or any of the select choices are changed - either programmatically or by the user).

We are using workspace storage for these settings, so they are set on a per workspace basis as the name implies. ~~There is no API available to clear them, but I have used this technique to clear all workspace knowledge from VSCode:~~

~~https://stackoverflow.com/a/71178197~~

Automated Tests

Directions for Reviewers

You'll want to have multiple deployments, multiple configurations and multiple credentials with which you can use for validation. On the credentials, you can use the same API key across multiple accounts, so just duplicate up a few to the same server. You'll want to have credentials for two different servers, but also multiple credentials for at least one of them.

As I mentioned in the overview, the last used settings are restored when the Home View is initialized. This allows you to be able to test and reproduce this cycle by using the pallet command: "Developer: Reload Windows" each time you want to test if the values you currently have will be restored.

To start with, you should just verify that after making some changes to the select boxes, you can reload the window and then see that those same values are restored.

After that works, you'll want to verify that the business logic that occurs when a user selects a deployment (and then we automatically select the configuration last used for the deployment or to setup the pre-deployment, along with then making sure the account selected matches the deployment URL - either by validating the currently selected one or by switching to the first in the filtered list), is not being used to initially set the values upon reload.

To do this:

select a different deployment and record which config and credential are selected.
Change both of them to something else.
Now reload the extension with the command: "Developer: Reload Windows"
Make sure the values persist from your last selection.
Then switch to a different deployment and notice everything possibly changing
Then switch back to that previous deployment and confirm that the business logic is in effect and that the last configuration used for deployment and the first matching account or currently matching account is selected.

There are also some edge conditions in which changes of available selection options are taken into account. For example, if a user selects a particular configuration, closes that workspace or VSCode itself, and then removes the configuration file they had selected physically from the filesystem, the initialization process is smart enough to simply fall back to business rules. However, it should be mentioned that we have an outstanding issues which need to handle when there are no longer configurations or when there are no matching credentials available that could be used with a deployment (#1280), so please do not expect that those conditions are handled yet as part of this PR.

There are a good number of combinations here, but by going through these a few times, you can pretty much validate that the implementation matches the intentions.

If you have any question of results, please reach out. For the most part, the validation of this PR is to make sure the values last used is restored ignoring those business rules, and the actual business rule validation is part of a different PR already merged.

Checklist

I have updated CHANGELOG.md to cover notable changes.

sagerb · 2024-04-13T20:04:31Z

extensions/vscode/src/views/homeView.ts

+  deploymentName: string;
+  configurationName: string;
+  credentialName: string;
+};


We store the selection state as an object within VSCode workspace storage

sagerb · 2024-04-13T20:18:02Z

extensions/vscode/src/views/homeView.ts

@@ -100,7 +109,7 @@ export class HomeViewProvider implements WebviewViewProvider {
          // are created within the webview this._context (i.e. inside media/main.js)
          case "initializing":
            // send back the data needed.
-            await this.refreshAll();
+            await this.refreshAll(true);


refreshAll has been updated to understand if it needs to send the initial state of the selections along with the refreshed data or not. This will only occur in response to the initializing request from the Vue App.

sagerb · 2024-04-13T20:19:09Z

extensions/vscode/src/views/homeView.ts

-              true,
-            );
+          case "navigate":
+            env.openExternal(Uri.parse(message.payload));
            break;


the commands "expanded" and "collapsed" have been refaced into the "updateDeploymentButtonExpanded" command.

sagerb · 2024-04-13T20:37:19Z

extensions/vscode/webviews/homeView/src/App.vue

@@ -51,6 +51,7 @@
          v-model="selectedDeployment"
          :options="deployments"
          :get-key="(d) => d.saveName"
+          @update:modelValue="onUpdateModelValueSelectedDeployment"


We switch to triggering off of the actual update, coming from the component, which is only raised when the user performs an action and is not raised when we change the v-model value programatically.

sagerb · 2024-04-13T20:38:10Z

extensions/vscode/webviews/homeView/src/App.vue

  updateCredentialsAndConfigurationForDeployment();
-});
+  updateParentViewSelectionState();
+};


Switching from the watch of the v-model value, so we can perform our business logic only when the user initiates the change (in contrast to us setting the v-model value as part of the initialization).

Very nice split.

sagerb · 2024-04-13T20:38:37Z

extensions/vscode/webviews/homeView/src/App.vue

@@ -261,7 +273,7 @@ const updateSelectedConfigurationByName = (
 ): boolean => {
  const previousSelectedConfig = selectedConfig.value;
  let selectedConfigTarget: Configuration | undefined = undefined;
-  if (selectedConfig.value) {
+  if (configurationName) {


This was a bug.

sagerb · 2024-04-13T20:39:29Z

extensions/vscode/webviews/homeView/src/App.vue

@@ -276,7 +288,7 @@ const updateSelectedConfigurationByName = (
 const updateSelectedCredentialByName = (credentialName?: string): boolean => {
  const previousSelectedAccount = selectedAccount.value;
  let selectedCredentialTarget: Account | undefined = undefined;
-  if (selectedAccount.value) {
+  if (credentialName) {


another bug.

sagerb · 2024-04-13T20:40:32Z

extensions/vscode/webviews/homeView/src/App.vue

-        // Always cause the re-calculation even if selected deployment didn't change
-        updateCredentialsAndConfigurationForDeployment();
+      if (payload.selectedDeploymentName) {
+        updateSelectedDeploymentByName(payload.selectedDeploymentName);


So if we have a selected value, we'll set it by name and cascade any business changes.

Really like that each of these utilize the updateSelectedDeploymentByName function (and similar) so we already had the logic for handling if the name isn't found in the list.

sagerb · 2024-04-13T20:40:44Z

extensions/vscode/webviews/homeView/src/App.vue

-        selectedConfig.value?.configurationName,
-      );
+      if (payload.selectedConfigurationName) {
+        updateSelectedConfigurationByName(payload.selectedConfigurationName);


same thought as above here.

sagerb · 2024-04-13T20:40:51Z

extensions/vscode/webviews/homeView/src/App.vue

      break;
    }
    case "refresh_credential_data": {
      const payload = JSON.parse(event.data.payload);
      accounts.value = payload.credentials;
-      updateSelectedCredentialByName(selectedAccount.value?.name);
+      if (payload.selectedCredentialName) {
+        updateSelectedCredentialByName(payload.selectedCredentialName);


dotNomad

This is working wonderfully. I'm glad we waited until the object work was complete.

I had a couple small comments, but running through your instructions for review I didn't hit anything that made me think there was a bug.

extensions/vscode/src/views/homeView.ts

dotNomad · 2024-04-16T22:20:07Z

extensions/vscode/webviews/homeView/src/App.vue

  updateCredentialsAndConfigurationForDeployment();
-});
+  updateParentViewSelectionState();
+};


Very nice split.

dotNomad · 2024-04-16T22:29:32Z

extensions/vscode/webviews/homeView/src/App.vue

-        // Always cause the re-calculation even if selected deployment didn't change
-        updateCredentialsAndConfigurationForDeployment();
+      if (payload.selectedDeploymentName) {
+        updateSelectedDeploymentByName(payload.selectedDeploymentName);


Really like that each of these utilize the updateSelectedDeploymentByName function (and similar) so we already had the logic for handling if the name isn't found in the list.

…isted-selection

initial working state

b0d3630

sagerb commented Apr 13, 2024

View reviewed changes

small naming refactor to avoid confusion

bac562e

sagerb commented Apr 13, 2024

View reviewed changes

sagerb marked this pull request as ready for review April 13, 2024 20:41

sagerb requested review from dotNomad, kgartland-rstudio, mmarchetti and tdstein as code owners April 13, 2024 20:41

dotNomad approved these changes Apr 16, 2024

View reviewed changes

PR recommendations

6d5984a

Base automatically changed from sagerb-update-extension-providers-to-have-consistent-initialization to main April 17, 2024 15:44

Merge remote-tracking branch 'origin/main' into sagerb-home-view-pers…

fa42a36

…isted-selection

sagerb merged commit a6e053f into main Apr 17, 2024
16 checks passed

sagerb deleted the sagerb-home-view-persisted-selection branch April 17, 2024 15:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Save / Restore selection and expansion state on extension load #1373

Save / Restore selection and expansion state on extension load #1373

sagerb commented Apr 12, 2024 •

edited

Loading

sagerb Apr 13, 2024

sagerb Apr 13, 2024

sagerb Apr 13, 2024

sagerb Apr 13, 2024

sagerb Apr 13, 2024

dotNomad Apr 16, 2024

sagerb Apr 13, 2024

sagerb Apr 13, 2024

sagerb Apr 13, 2024

dotNomad Apr 16, 2024

sagerb Apr 13, 2024

sagerb Apr 13, 2024

dotNomad left a comment

dotNomad Apr 16, 2024

dotNomad Apr 16, 2024

Save / Restore selection and expansion state on extension load #1373

Save / Restore selection and expansion state on extension load #1373

Conversation

sagerb commented Apr 12, 2024 • edited Loading

Intent

Type of Change

Approach

Automated Tests

Directions for Reviewers

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotNomad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sagerb commented Apr 12, 2024 •

edited

Loading