Skip to content

Improving the azure browse filter dropdown logic #18281

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

Merged
merged 10 commits into from
Oct 25, 2024

Conversation

Benjin
Copy link
Contributor

@Benjin Benjin commented Oct 18, 2024

Refining and consolidating the logic to have more intuitive default logic. The overall logic is now:

  • server always selects the first one in the list to make sure it's always populated, unless the user has explicitly cleared it
  • all others default to no filter
  • dropdowns are automatically scoped by any upstream filters that are set (subscriptions/resource groups/locations/servers)
  • if a value is already selected and the upstream filters change, the selection stays if it's still in-filter or is cleared if not

Also adding several telemetry more points around Azure load times and errors

Note: because contents of subscriptions are loaded asynchronously, different servers may end up being selected each time depending on which subscriptions finish first. I thought it would be more confusing to swap the selection around to (e.g.) the first alphabetically as new entries loaded, so /shrug. The other dropdowns don't have that issue because they aren't set to always make a selection when available.

Copy link

github-actions bot commented Oct 18, 2024

PR Changes

Category Main Branch PR Branch Difference
Code Coverage 50.51% 0.00% $${\color{lightgreen} -50.51\% }$$
VSIX Size 11656 KB 11659 KB $${\color{lightgreen} 3 KB \space (0\%) }$$
Webview Bundle Size 2988 KB 2984 KB $${\color{lightgreen} -4 KB \space (0\%) }$$

@Benjin Benjin marked this pull request as ready for review October 22, 2024 19:23
@Benjin Benjin requested a review from a user October 22, 2024 19:23
@Benjin Benjin requested a review from lewis-sanchez as a code owner October 22, 2024 19:23
@@ -1138,14 +1138,16 @@ export class ConnectionDialogWebviewController extends ReactWebviewPanelControll
string[] | undefined
>(azureSubscriptionFilterConfigKey) !== undefined;

const startTime = Date.now();
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should create a helper function to track time. It would return another function that, when called, automatically measures the elapsed time and sends the action event with the mentioned properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like this thought. I'll make an issue to add some util stuff like that.

Copy link
Member

@kburtram kburtram left a comment

Choose a reason for hiding this comment

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

Looks like a solid improvement to me!

@Benjin Benjin merged commit 0eabc8a into main Oct 25, 2024
3 of 4 checks passed
@Benjin Benjin deleted the dev/benjin/improveAzureFiltering branch October 25, 2024 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants