feat: update vscode UI toolkit to fast v2 #273

chrisdholt · 2021-10-20T00:28:37Z

Link to relevant issue

This pull request resolves #261
This pull request resolves #193

Description of changes

This PR updates the VSCode Webview UI Toolkit to leverage FAST Foundation v2. In FAST Foundation v2 there are a number of improvements which add additional ergonomics and support to component libraries and design systems. The following details provide insight into the changes.

Foundation Element

One of the most significant changes for FAST Foundation v2 is the added support of FoundationElement and Configuration which provides a method of registration and additional configuration abilities by consumers of design systems and/or component libraries such as this.

Templates and styles are functions

One part of this change is that templates and therefore styles in @microsoft/fast-foundation are now functions with arguments for a context and definition object.

Context

While not a robust accounting of all features available, it may be helpful to know that context enables the easy nesting of components while still enabling the configuration of unique (dynamic) prefixes), providing greater extensibility. This allows FAST (and other DS authors such as vscode) to nest components without needing to know the finale web component prefix. Consider the below template and styles:

const myFancyTemplate = (context, definition) => html`
    <template id="elementRoot">
         <slot></slot>
         <${context.tagFor(Button)}>${x => x.buttonText}</${context.tagFor(Button)}>
    </template>

const myFancyStyles = (context, definition) => css`
    :host {
        display: block;
    }
    
    ${context.tagFor(Button)} {
        // my nested button styles
    }
`;

When registered by default, the output above will be:

<my-fancy-element id="elementRoot">
     <slot></slot>
    <vscode-button>Button Text</vscode-button>
</my-fancy-element>

    :host {
        display: block;
    }
    
    vscode-button {
        // my nested button styles
    }

Given a different prefix at registration time, such as foo, the button element and it's declaration in the styles file would reference foo-button.

Definition

Definition provides another layer of customization at configuration time. Currently at the Fast Foundation level, the primary use of definition is to enumerate slots which should support "default slotted content". Rather than relying on slotted content to be baked into the fast-foundation templates, libraries now define the default slotted content during composition. Additionally, users of these libraries/design systems are now able to register the elements with their own instance of default slotted content. Scenarios here may be the desire to swap out the default SVG for dropdown or to include a different SVG for the checkbox checked indicator.

Registration

Previously, the default export was elements themselves and you would need to import and reference them to prevent tree shaking and ensure that they were available for use. Now, the export here is a registration for the element which enables its use.

More details on registration and configuration can be found here: https://www.fast.design/docs/design-systems/fast-frame#create-a-designsystem

Registration provides an opportunity to add global configurations across an application. Consider a scenario where (for some reason), an extension author needs to run the shadow DOM in closed mode - this, as well as template replacement, style changes, updates to default slotted content, etc...are all enable. Find out more about configuration during registration here: https://www.fast.design/docs/design-systems/fast-frame/#configuring-components-during-registration

As always, CSS can be used to target elements globally, slotted content will project and replace any default slotted content, and everything else about HTML elements continue to work as expected.

Again, this isn't exhaustive, but hopefully this provides context for some clear deltas between v1 and v2.

Link to forked docs site

TBD...

Please see CONTRIBUTING.md for directions on how this can be done.

…e exports

… foundation

src/badge/badge.styles.ts

src/badge/index.ts

src/custom-elements.ts

src/option/option.styles.ts

src/divider/index.ts

src/vscode-design-system.ts

chrisdholt · 2021-10-20T00:56:22Z

@hawkticehurst I'll look into the build failure. Something is not working with eslint. I'm going to look into that. As I mentioned previously I think, the eslint errors shouldn't exist. Something odd is definitely going on here. I have a no-unused-vars error for all the types I've added and used :).

chrisdholt · 2021-10-20T03:50:38Z

@hawkticehurst I'll look into the build failure. Something is not working with eslint. I'm going to look into that. As I mentioned previously I think, the eslint errors shouldn't exist. Something odd is definitely going on here. I have a no-unused-vars error for all the types I've added and used :).

eslint resolved - significant version mismatch between that used by the current FAST eslint rules and the repo. I've resolved to a similar version for now. We've had an rfc out (microsoft/fast#4468) for improving the rules and that would likely include changes to the versions; we can likely prioritize that. Seems like there's been some good improvements to eslint :).

The build is related to a type issue that shouldn't be present, so I'll look into that.

chrisdholt · 2021-10-20T04:42:42Z

@hawkticehurst I'll look into the build failure. Something is not working with eslint. I'm going to look into that. As I mentioned previously I think, the eslint errors shouldn't exist. Something odd is definitely going on here. I have a no-unused-vars error for all the types I've added and used :).

eslint resolved - significant version mismatch between that used by the current FAST eslint rules and the repo. I've resolved to a similar version for now. We've had an rfc out (microsoft/fast#4468) for improving the rules and that would likely include changes to the versions; we can likely prioritize that. Seems like there's been some good improvements to eslint :).

The build is related to a type issue that shouldn't be present, so I'll look into that.

Build resolved!

The issue the build was hitting was due to a type conflict for the compose function and its override function. We're tracking an issue for the error, but I was surprised to seeing it error in this context. The delta here is that there is the default definition for Foundation and an Override definition. In one of those, a type can be undefined so there is a mismatch. We just need to do some type work.

To accommodate the issue, instead of dropping ALL currently enabled strict modes, I just turned this instance off.

hawkticehurst · 2021-10-20T21:58:04Z

First off thank you again so much for this PR! 🎉🎊

Your description of changes was also super well written and helpful for understanding some of the major updates from foundation v1 to v2 so thank you for that!

hawkticehurst

Looks great thus far! Left a few random comments/questions.

I'll wait for some of the changes discussed in your comments to land and then do a final review/approval.

tsconfig.json

package.json

src/index-rollup.ts

src/dropdown/index.ts

src/option/index.ts

src/dropdown/index.ts

chrisdholt · 2021-10-21T05:05:49Z

Just an FYI - the reason that typescript needs to be pinned at 4.3.5 is outlined in this issue here: microsoft/TypeScript#46456.

src/vscode-design-system.ts

daviddossett

Looks great to me! I'll defer to Hawk for the final sign off 🤘

hawkticehurst · 2021-10-21T17:55:50Z

@chrisdholt Cloned your PR locally to do some testing and looks like there are a couple of issues.

Storybook

Data grid component not rendering
Dropdown stories have an error
Panels stories have an error

All Components Sample

Note: We currently use the all-components sample extension to test the toolkit inside a real VS Code environment (we've found there are times when Storybook and Webviews have discrepancies in rendering).

We have future plans of creating a proper automated testing pipeline, but for now, we just have to manually build the toolkit dist directory and then copy/paste/replace it with node_modules/@vscode/webview-ui-toolkit/dist inside the all-components sample.

Data grid component not rendering
Dropdown and panel components seem to be fine!

chrisdholt · 2021-10-21T18:00:47Z

@chrisdholt Cloned your PR locally to do some testing and looks like there are a couple of issues.

Storybook

Data grid component not rendering

Dropdown stories have an error

Panels stories have an error

All Components Sample

Note: We currently use the all-components sample extension to test the toolkit inside a real VS Code environment (we've found there are times when Storybook and Webviews have discrepancies in rendering).

We have future plans of creating a proper automated testing pipeline, but for now, we just have to manually build the toolkit dist directory and then copy/paste/replace it with node_modules/@vscode/webview-ui-toolkit/dist inside the all-components sample.

Data grid component not rendering

Dropdown and panel components seem to be fine!

I'll dig in and see what's going on here :)

…rors due to renames

chrisdholt · 2021-10-21T18:58:43Z

@chrisdholt Cloned your PR locally to do some testing and looks like there are a couple of issues.
Storybook

Data grid component not rendering

Dropdown stories have an error

Panels stories have an error

All Components Sample
Note: We currently use the all-components sample extension to test the toolkit inside a real VS Code environment (we've found there are times when Storybook and Webviews have discrepancies in rendering).
We have future plans of creating a proper automated testing pipeline, but for now, we just have to manually build the toolkit dist directory and then copy/paste/replace it with node_modules/@vscode/webview-ui-toolkit/dist inside the all-components sample.

Data grid component not rendering

Dropdown and panel components seem to be fine!

I'll dig in and see what's going on here :)

Got these resolved - a couple of type issues and then the Data Grid issue was related to needing to provide a mapping to register the base class. Should be all good!

hawkticehurst · 2021-10-21T20:32:43Z

Got these resolved - a couple of type issues and then the Data Grid issue was related to needing to provide a mapping to register the base class. Should be all good!

Yay! Thank you so much!

I'll validate the changes, do a final PR review, and assuming all looks good I'm ready to approve and merge!

hawkticehurst · 2021-10-21T21:39:12Z

@chrisdholt Looks like there are still issues with the data grid in the sample extension.

Did this update change how the data grid should be instantiated?

Currently, the default data grid is created with the following code in the sample extension.

<vscode-data-grid id="default-grid" grid-template-columns="1fr 1fr 1fr 1fr" aria-label="Default"></vscode-data-grid>

const defaultDataGrid = document.getElementById("default-grid");
  defaultDataGrid.rowsData = [
    {
      column1: "Cell Data",
      column2: "Cell Data",
      column3: "Cell Data",
      column4: "Cell Data",
    },
    {
      column1: "Cell Data",
      column2: "Cell Data",
      column3: "Cell Data",
      column4: "Cell Data",
    },
    {
      column1: "Cell Data",
      column2: "Cell Data",
      column3: "Cell Data",
      column4: "Cell Data",
    },
  ];

chrisdholt · 2021-10-21T21:40:54Z

@chrisdholt Looks like there are still issues with the data grid in the sample extension.

Did this update change how the data grid should be instantiated?

Currently, the default data grid is created with the following code in the sample extension.

<vscode-data-grid id="default-grid" grid-template-columns="1fr 1fr 1fr 1fr" aria-label="Default"></vscode-data-grid>

const defaultDataGrid = document.getElementById("default-grid");
  defaultDataGrid.rowsData = [
    {
      column1: "Cell Data",
      column2: "Cell Data",
      column3: "Cell Data",
      column4: "Cell Data",
    },
    {
      column1: "Cell Data",
      column2: "Cell Data",
      column3: "Cell Data",
      column4: "Cell Data",
    },
    {
      column1: "Cell Data",
      column2: "Cell Data",
      column3: "Cell Data",
      column4: "Cell Data",
    },
  ];

How can I run/test this on my end? I believe what I've added should've addressed this, but would be good to know if I can run locally.

hawkticehurst · 2021-10-21T21:49:34Z

How can I run/test this on my end? I believe what I've added should've addressed this, but would be good to know if I can run locally.

Clone this repo: https://github.com/microsoft/vscode-webview-ui-toolkit-samples
This repo holds a bunch of self-contained sample extensions so open the all-components directory inside VS Code (not the root directory)
npm install to install sample dependencies
Open your fork of the main toolkit repo and dev branch for this PR
Run npm run build
Copy the generated dist folder
Go back to the all-components sample and replace node_modules/@vscode/webview-ui-toolkit/dist with the dist you just copied
Press F5 on your keyboard to open a development host window that has the sample extension loaded
Open the VS Code command palette (ctrl + shift + p or cmd + shift + p on Mac)
Type "Webview UI Toolkit: All Components" and click enter (this will run/render the sample webview)
If you want to be able to inspect the elements, open the command palette again and type "Developer: Open Webview Developer Tools"

Oh also as you look at the code inside the all-components directory:

./media/main.js is where the JS file lives that defines rowsData on the data grids
./src/panels/demos/data-grid.ts is where the HTML lives

hawkticehurst

Left a few more comments on things to fix/discuss, but beyond that looks fantastic! I'm ready to merge once those are all addressed! 🎉

src/index-rollup.ts

docs/api-report.md

package.json

src/custom-elements.ts

src/dropdown/index.ts

src/link/index.ts

src/option/index.ts

chrisdholt · 2021-10-22T02:20:03Z

How can I run/test this on my end? I believe what I've added should've addressed this, but would be good to know if I can run locally.

Clone this repo: https://github.com/microsoft/vscode-webview-ui-toolkit-samples

This repo holds a bunch of self-contained sample extensions so open the all-components directory inside VS Code (not the root directory)

npm install to install sample dependencies

Open your fork of the main toolkit repo and dev branch for this PR

Run npm run build

Copy the generated dist folder

Go back to the all-components sample and replace node_modules/@vscode/webview-ui-toolkit/dist with the dist you just copied

Press F5 on your keyboard to open a development host window that has the sample extension loaded

Open the VS Code command palette (ctrl + shift + p or cmd + shift + p on Mac)

Type "Webview UI Toolkit: All Components" and click enter (this will run/render the sample webview)

If you want to be able to inspect the elements, open the command palette again and type "Developer: Open Webview Developer Tools"

Oh also as you look at the code inside the all-components directory:

./media/main.js is where the JS file lives that defines rowsData on the data grids

./src/panels/demos/data-grid.ts is where the HTML lives

This is super cool!

OK, I know what was causing the data grid issue and I don't know why I didn't think of it. The issue is with the regex for the template, we had to modify it to account for spacing w/r/t attributes/property bindings in templates. If you were to look at the DOM you'd see the Data Grid Cell template being absolutely mangled. I've updated the regex and ran things and everything looks gravy :)

chrisdholt · 2021-10-22T02:34:00Z

package.json

@@ -30,9 +30,8 @@
 		"test": "jest --verbose --coverage"
 	},
 	"dependencies": {
-		"@microsoft/fast-element": "^1.2.0",
-		"@microsoft/fast-foundation": "^1.24.7",
-		"lodash-es": "^4.17.21"


Bye bye! FYI @connor4312 👍

chrisdholt · 2021-10-22T02:34:46Z

OK, we're all resolved I think 👍

hawkticehurst · 2021-10-22T21:11:05Z

Alright did my final checks and I'm ready to merge! A pleasant surprise is I just realized this update also fixed the bug discussed in #193. 🙂

hawkticehurst · 2021-10-22T21:13:17Z

Wooo! We're merged! Once more, thank you so much for your work on this @chrisdholt!!! 🎉🎊

I'll follow up shortly by publishing a new toolkit version to NPM.

chrisdholt added 4 commits October 19, 2021 15:51

update to fast-foundation v2 APIs, create vscode-design-system, updat…

272e783

…e exports

fix docs and ensure name is baseName in registration

f5be6f9

add explict class to register from for components directly inheriting…

7d74db8

… foundation

add default slotted content

b00dddc