Adding advanced connection settings pane to connection dialog

[Benjin]

• Refactoring connection dialog form slightly
• Generating form items from STS call (paired with some fixups in Fixing up missing info in the capabilities call sqltoolsservice#2386)
• Adding help text to each item, where available

Screen.Recording.2024-08-23.093545.mp4

[Benjin]

note to self: increase the linewidth for prettier to avoid unnecessary wrapping

[Benjin]

Pull FormContextProps<T>, FormEvent<T>, and baseclass for ConnectionDialogReducers to Form-dedicated file

[kburtram]

@Benjin in terms of UX, I was expecting the advanced options to be an additional tab, like Connection String. From the video the UX looks good to me (I'll sync the branch and test it out). Though I'm interested if this "dialog on top of dialog within a document" pattern is what we want to follow across the product. No concerns with this for the initial check-in, but we should do a UX review prior to public preview.

[Benjin]

@kburtram Agreed that we should have Janki look at this before we go to public preview. I put it as a drawer in the same tab (for now) for two reasons:

1. the two existing tabs - "Parameters" and "Connection String" - operate independently instead of converting between connection string form and per-property form of the same connection details. I think the tabs should have consistent behavior (all tabs reflect properties/connection string of the same connection, or each tab operates independently), but don't have a preference as to which. Something to chat with Janki about.

2. The ADS connection dialog throws a pane over the main options, so this matches that. Just inspiration, though; not that we should be just copying ADS UX at every turn.

[github-actions]

VSIX Size Comparison

• Main branch VSIX size: 11256 KB
• PR branch VSIX size: 11264 KB
• Size difference: 8 KB (0%)

React Webview Bundle Size Comparison

• Main branch bundle size: 856 KB
• PR branch bundle size: 872 KB
• Size difference: 16 KB (1%)

[croblesm]

Hi @Benjin,

This looks promising, however I have some feedback:

• Change the Parameter vs Connection string option. Use a radio button for this option, and make Parameter the default and first option.
• Change the Advanced and Connect button positioning, I would suggest putting them side by side.
• Reorder the Advanced options, show the most common (General) options first
• Test connection button is missing, but I'm not sure if you have that functionality ready in your backend
• Cancel button is missing

This is a concept design from Janki:

[kburtram]

I made it through about half the code. Will take a look at the rest later today & will build/test the branch. Please work with @aasimkhan30 to get a sign-off since he is most familiar with some of the code you're updating.

[kburtram]

it seems like this validation logic could be wired up more automatically using the "required" metadata property? Since there's only a few properties I'm not sure it's worth refactoring though.

[Benjin]

I definitely something worth looking into as the generic form structure gets refined. It might need more prodding to correctly handle fields that are "conditionally required", like username, which is required, but only if auth is set to SqlLogin. Lemme see what I can do.

[kburtram]

@Benjin makes sense on the design rationale. I think this UI looks good to me; it just wasn't what I was expecting. I'm interested in using these first UIs as an opportunity to establish UX patterns to copy in other features and this seems like a good 'test case.' Though I'd definitely put this feedback in the "open a follow-up task to do in a separate PR" so that we can get this core functionality into next release.

[aasimkhan30]

Having a drawer over the recent connections feels a bit awkward since the recent connections already resemble a drawer. Initially, I considered using toggle buttons for the parameters and connection string, along with tabs for the basic and advanced fields.

Example of toggle button

Unfortunately, fluent doesn't support this ui component

[Benjin]

Having a drawer over the recent connections feels a bit awkward since the recent connections already resemble a drawer. Initially, I considered using toggle buttons for the parameters and connection string, along with tabs for the basic and advanced fields. Unfortunately, fluent doesn't support this ui component

@aasimkhan30 and I chatted offline, and I'll raise his ideas + concerns with Janki when we meet on Monday to iron out the differences between the mockups and what VSCode webviews can do.