Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Setup Storybook for testing UI components #1497
Setup Storybook for testing UI components #1497
Changes from 2 commits
6018eba
2211e23
d15e388
baf130d
9383b03
a38a035
6fdc632
2ee46cf
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of having our own file that we have to manage/keep up-to date etc., is there a way to get everything from VSCode automatically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might be able to generate it from the VSCode source code, but I don't think we can import it from VSCode itself.
The colors are defined here and the styles are created here. They are then injected into the WebView using this on the host side and this on the WebView side.
I think it will be a lot more work to generate the CSS variables from this than to copy the variables from VSCode once in a while.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah that's a shame. I suspected it would be a can of worms, but I thought I'd ask anyway 😂
In that case I think we should have some docs to tell people about this (maybe in the storybook part in the README?) and how to update them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps I can also add it to an Overview page in Storybook itself and refer to that from the CONTRIBUTING documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just had a brainwave! Feel free to ignore this if you want (totally up to you), but the actual toolkit has a storybook, so perhaps this is something they've solved already? See https://github.com/microsoft/vscode-webview-ui-toolkit/tree/main/.storybook
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did see whether it was possible to replicate the way they are using the VSCode variables, but they are using a completely custom system. They have a system of "design tokens" (https://github.com/microsoft/vscode-webview-ui-toolkit/blob/6fc339ce26e7da15b9ba97c063f56d8a4eae85ad/src/design-tokens.ts) which specify a variable and a default. These are then used in the styles and will automatically use either the variable or the default. I don't think that is going to work for us since we'd need to modify the way we write the CSS files and set defaults for all variables we use. It could also result in having different defaults than the toolkit, so it might also get out of sync at some point.