Skip to content

Commit 0eabc8a

Browse files
BenjinBenjin
authored
Improving the azure browse filter dropdown logic (#18281)
* consolidating filter logic * checkpoint * correcting filter autoselection logic * cleanup * resetting list of servers when subscription filter changes * swapping default selection flag for enum * light refactoring * updating comment
1 parent 5bf629f commit 0eabc8a

File tree

6 files changed

+264
-168
lines changed

6 files changed

+264
-168
lines changed

‎src/connectionconfig/connectionDialogWebviewController.ts

+25-8
Original file line numberDiff line numberDiff line change
@@ -1138,14 +1138,16 @@ export class ConnectionDialogWebviewController extends ReactWebviewPanelControll
11381138
string[] | undefined
11391139
>(azureSubscriptionFilterConfigKey) !== undefined;
11401140

1141+
const startTime = Date.now();
1142+
11411143
this._azureSubscriptions = new Map(
11421144
(await auth.getSubscriptions(shouldUseFilter)).map((s) => [
11431145
s.subscriptionId,
11441146
s,
11451147
]),
11461148
);
11471149
const tenantSubMap = this.groupBy<string, AzureSubscription>(
1148-
await auth.getSubscriptions(shouldUseFilter),
1150+
Array.from(this._azureSubscriptions.values()),
11491151
"tenantId",
11501152
); // TODO: replace with Object.groupBy once ES2024 is supported
11511153

@@ -1164,13 +1166,31 @@ export class ConnectionDialogWebviewController extends ReactWebviewPanelControll
11641166
state.azureSubscriptions = subs;
11651167
state.loadingAzureSubscriptionsStatus = ApiStatus.Loaded;
11661168

1169+
sendActionEvent(
1170+
TelemetryViews.ConnectionDialog,
1171+
TelemetryActions.LoadAzureSubscriptions,
1172+
undefined, // additionalProperties
1173+
{
1174+
subscriptionCount: subs.length,
1175+
msToLoadSubscriptions: Date.now() - startTime,
1176+
},
1177+
);
1178+
11671179
this.updateState();
11681180

11691181
return tenantSubMap;
11701182
} catch (error) {
11711183
state.formError = l10n.t("Error loading Azure subscriptions.");
11721184
state.loadingAzureSubscriptionsStatus = ApiStatus.Error;
11731185
console.error(state.formError + "\n" + getErrorMessage(error));
1186+
1187+
sendErrorEvent(
1188+
TelemetryViews.ConnectionDialog,
1189+
TelemetryActions.LoadAzureSubscriptions,
1190+
error,
1191+
false, // includeErrorMessage
1192+
);
1193+
11741194
return undefined;
11751195
}
11761196
}
@@ -1179,7 +1199,6 @@ export class ConnectionDialogWebviewController extends ReactWebviewPanelControll
11791199
state: ConnectionDialogWebviewState,
11801200
): Promise<void> {
11811201
try {
1182-
const startTime = Date.now();
11831202
const tenantSubMap = await this.loadAzureSubscriptions(state);
11841203

11851204
if (!tenantSubMap) {
@@ -1192,8 +1211,11 @@ export class ConnectionDialogWebviewController extends ReactWebviewPanelControll
11921211
);
11931212
} else {
11941213
state.loadingAzureServersStatus = ApiStatus.Loading;
1214+
state.azureServers = [];
11951215
this.updateState();
11961216

1217+
const startTime = Date.now();
1218+
11971219
const promiseArray: Promise<void>[] = [];
11981220
for (const t of tenantSubMap.keys()) {
11991221
for (const s of tenantSubMap.get(t)) {
@@ -1229,12 +1251,7 @@ export class ConnectionDialogWebviewController extends ReactWebviewPanelControll
12291251
TelemetryViews.ConnectionDialog,
12301252
TelemetryActions.LoadAzureServers,
12311253
error,
1232-
true, // includeErrorMessage
1233-
undefined, // errorCode
1234-
undefined, // errorType
1235-
{
1236-
connectionInputType: this.state.selectedInputMode,
1237-
},
1254+
false, // includeErrorMessage
12381255
);
12391256

12401257
return;

‎src/reactviews/common/comboboxHelper.ts

+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
/** Behavior for how the default selection is determined */
7+
export enum DefaultSelectionMode {
8+
/** If there are any options, the first is always selected. Otherwise, selects nothing. */
9+
SelectFirstIfAny,
10+
/** Always selects nothing, regardless of if there are available options */
11+
AlwaysSelectNone,
12+
/** Selects the only option if there's only one. Otherwise (many or no options) selects nothing. */
13+
SelectOnlyOrNone,
14+
}
15+
16+
export function updateComboboxSelection(
17+
/** current selected (valid) option */
18+
selected: string | undefined,
19+
/** callback to set the selected (valid) option */
20+
setSelected: (s: string | undefined) => void,
21+
/** callback to set the displayed value (not guaranteed to be valid if the user has manually typed something) */
22+
setValue: (v: string) => void,
23+
/** list of valid options */
24+
optionList: string[],
25+
/** behavior for choosing the default selected value */
26+
defaultSelectionMode: DefaultSelectionMode = DefaultSelectionMode.AlwaysSelectNone,
27+
) {
28+
// if there is no current selection or if the current selection is no longer in the list of options (due to filter changes),
29+
// then select the only option if there is only one option, then make a default selection according to specified `defaultSelectionMode`
30+
31+
if (
32+
selected === undefined ||
33+
(selected && !optionList.includes(selected))
34+
) {
35+
let optionToSelect: string | undefined = undefined;
36+
37+
if (optionList.length > 0) {
38+
switch (defaultSelectionMode) {
39+
case DefaultSelectionMode.SelectFirstIfAny:
40+
optionToSelect =
41+
optionList.length > 0 ? optionList[0] : undefined;
42+
break;
43+
case DefaultSelectionMode.SelectOnlyOrNone:
44+
optionToSelect =
45+
optionList.length === 1 ? optionList[0] : undefined;
46+
break;
47+
case DefaultSelectionMode.AlwaysSelectNone:
48+
default:
49+
optionToSelect = undefined;
50+
}
51+
}
52+
53+
setSelected(optionToSelect); // selected value's unselected state should be undefined
54+
setValue(optionToSelect ?? ""); // displayed value's unselected state should be an empty string
55+
}
56+
}

‎src/reactviews/common/utils.ts

+5
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,8 @@ export function themeType(theme: Theme): string {
4949
}
5050
return themeType;
5151
}
52+
53+
/** Removes duplicate values from an array */
54+
export function removeDuplicates<T>(array: T[]): T[] {
55+
return Array.from(new Set(array));
56+
}

‎src/reactviews/pages/ConnectionDialog/AzureFilterCombobox.component.tsx

+132
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import {
7+
Combobox,
8+
ComboboxProps,
9+
Field,
10+
makeStyles,
11+
OptionOnSelectData,
12+
SelectionEvents,
13+
Option,
14+
} from "@fluentui/react-components";
15+
import { useFormStyles } from "../../common/forms/form.component";
16+
import { useEffect, useState } from "react";
17+
18+
const useFieldDecorationStyles = makeStyles({
19+
decoration: {
20+
display: "flex",
21+
alignItems: "center",
22+
columnGap: "4px",
23+
},
24+
});
25+
26+
export const AzureFilterCombobox = ({
27+
label,
28+
required,
29+
clearable,
30+
content,
31+
decoration,
32+
props,
33+
}: {
34+
label: string;
35+
required?: boolean;
36+
clearable?: boolean;
37+
content: {
38+
/** list of valid values for the combo box */
39+
valueList: string[];
40+
/** currently-selected value from `valueList` */
41+
selection?: string;
42+
/** callback when the user has selected a value from `valueList` */
43+
setSelection: (value: string | undefined) => void;
44+
/** currently-entered text in the combox, may not be a valid selection value if the user is typing */
45+
value: string;
46+
/** callback when the user types in the combobox */
47+
setValue: (value: string) => void;
48+
/** placeholder text for the combobox */
49+
placeholder?: string;
50+
/** message displayed if focus leaves this combobox and `value` is not a valid value from `valueList` */
51+
invalidOptionErrorMessage: string;
52+
};
53+
decoration?: JSX.Element;
54+
props?: Partial<ComboboxProps>;
55+
}) => {
56+
const formStyles = useFormStyles();
57+
const decorationStyles = useFieldDecorationStyles();
58+
const [validationMessage, setValidationMessage] = useState<string>("");
59+
60+
// clear validation message as soon as value is valid
61+
useEffect(() => {
62+
if (content.valueList.includes(content.value)) {
63+
setValidationMessage("");
64+
}
65+
}, [content.value]);
66+
67+
// only display validation error if focus leaves the field and the value is not valid
68+
const onBlur = () => {
69+
if (content.value) {
70+
setValidationMessage(
71+
content.valueList.includes(content.value)
72+
? ""
73+
: content.invalidOptionErrorMessage,
74+
);
75+
}
76+
};
77+
78+
const onOptionSelect: (
79+
_: SelectionEvents,
80+
data: OptionOnSelectData,
81+
) => void = (_, data: OptionOnSelectData) => {
82+
content.setSelection(
83+
data.selectedOptions.length > 0 ? data.selectedOptions[0] : "",
84+
);
85+
content.setValue(data.optionText ?? "");
86+
};
87+
88+
function onInput(ev: React.ChangeEvent<HTMLInputElement>) {
89+
content.setValue(ev.target.value);
90+
}
91+
92+
return (
93+
<div className={formStyles.formComponentDiv}>
94+
<Field
95+
label={
96+
decoration ? (
97+
<div className={decorationStyles.decoration}>
98+
{label}
99+
{decoration}
100+
</div>
101+
) : (
102+
label
103+
)
104+
}
105+
orientation="horizontal"
106+
required={required}
107+
validationMessage={validationMessage}
108+
onBlur={onBlur}
109+
>
110+
<Combobox
111+
{...props}
112+
value={content.value}
113+
selectedOptions={
114+
content.selection ? [content.selection] : []
115+
}
116+
onInput={onInput}
117+
onOptionSelect={onOptionSelect}
118+
placeholder={content.placeholder}
119+
clearable={clearable}
120+
>
121+
{content.valueList.map((val, idx) => {
122+
return (
123+
<Option key={idx} value={val}>
124+
{val}
125+
</Option>
126+
);
127+
})}
128+
</Combobox>
129+
</Field>
130+
</div>
131+
);
132+
};

0 commit comments

Comments
 (0)