vscode-elements / elements

Web component library for developing Visual Studio Code extensions
https://vscode-elements.github.io
MIT License
156 stars 27 forks source link

Few issues when migrating open source of Cloud Foundry Tools #163

Closed veredcon closed 1 month ago

veredcon commented 1 month ago

Hi Adam,

This is a really cool and useful library that we would like to adopt in SAP instead of the deprecated one. Thank you for developing it!

After trying to adopt it in some open source we have I encountered few issues: https://github.com/SAP/cloud-foundry-tools/pull/296

  1. codicons are not presented well when inside a vscode-element control e.g.: image The paste icon at the end of the textfield cannot be displayed (if it is just outside of the vscode-element there is no issue)
  2. You can see I added some styles of Link component, If there was a vscode-link (like in https://github.com/estruyf/vscode-community-ui-toolkit/blob/main/src/link/index.ts) one would not have to take care of it...
  3. I have an issue migrating the vscode-single select and I don't understand why, I tried many things we have there 2 dropdowns that needs to be populated with the selected org and space once they appear.. for some reason I get the selected org as expected (option is selected) but not the selected space (the selected space is correct but not rendered in the spaces dropdown)..
  4. Also I noticed some errors in the console image

I would appreciate your assistance in adopting this lib so it could be chosen for the rest of our SAP repositories using the old webview toolkit.

Thanks!

bendera commented 1 month ago

Hi Vered,

I can't answer all of your questions yet, but what I've found so far:

  1. The icon component isn't imported. The id="vscode-codicon-stylesheet" attribute is missing from the CSS link. See for examples: https://vscode-elements.github.io/components/icon/#basic-example
veredcon commented 1 month ago

Hi @bendera

Thank you so much for your prompt reply and the time for investing in this library! I see, when I add this specific id I do get the icon when running only frontend locally, but as the id is not maintained in the html after vite build it is not displayed when running the whole extension. (When I add it manually to the html dist I can see the icon). I'm wondering why this specific id is required because it wasn't necessary before and it makes it harder to smoothly migrate. For example without this specific stylesheet id I don't have an issue rendering this:

<span class="codicon codicon-clippy"></span></span>
bendera commented 1 month ago

For the use of icon font in shadow dom the CSS needs to be loaded both in shadow dom and global dom. The Icon component can find the proper CSS based on the id. Another solution would be finding the CSS by the filename, but it is not guaranteed that the filename will be "codicon.css". With the id attribute the developer can express explicitly "this is the codicon CSS".

Anyway, it's okay to use the codicon with native <span> tags, the Icon component itself is just a wrapper. Although, the action-icon mode is a somewhat useful feature.

bendera commented 1 month ago

I have an issue migrating the vscode-single select and I don't understand why, I tried many things we have there 2 dropdowns that needs to be populated with the selected org and space once they appear.. for some reason I get the selected org as expected (option is selected) but not the selected space (the selected space is correct but not rendered in the spaces dropdown)..

Could you please attach a minimalistic reproducible example?

bendera commented 1 month ago

You can see I added some styles of Link component, If there was a vscode-link (like in https://github.com/estruyf/vscode-community-ui-toolkit/blob/main/src/link/index.ts) one would not have to take care of it...

Webview UI Toolkit defines its own theme, while Elements assumes the default styles provided by VSCode are available. Add the following styles to your HTML and the theme variables during development.

<style id="_defaultStyles">
    html {
        scrollbar-color: var(--vscode-scrollbarSlider-background) var(--vscode-editor-background);
    }

    body {
        overscroll-behavior-x: none;
        background-color: transparent;
        color: var(--vscode-editor-foreground);
        font-family: var(--vscode-font-family);
        font-weight: var(--vscode-font-weight);
        font-size: var(--vscode-font-size);
        margin: 0;
        padding: 0 20px;
    }

    img, video {
        max-width: 100%;
        max-height: 100%;
    }

    a, a code {
        color: var(--vscode-textLink-foreground);
    }

    p > a {
        text-decoration: var(--text-link-decoration);
    }

    a:hover {
        color: var(--vscode-textLink-activeForeground);
    }

    a:focus,
    input:focus,
    select:focus,
    textarea:focus {
        outline: 1px solid -webkit-focus-ring-color;
        outline-offset: -1px;
    }

    code {
        font-family: var(--monaco-monospace-font);
        color: var(--vscode-textPreformat-foreground);
        background-color: var(--vscode-textPreformat-background);
        padding: 1px 3px;
        border-radius: 4px;
    }

    pre code {
        padding: 0;
    }

    blockquote {
        background: var(--vscode-textBlockQuote-background);
        border-color: var(--vscode-textBlockQuote-border);
    }

    kbd {
        background-color: var(--vscode-keybindingLabel-background);
        color: var(--vscode-keybindingLabel-foreground);
        border-style: solid;
        border-width: 1px;
        border-radius: 3px;
        border-color: var(--vscode-keybindingLabel-border);
        border-bottom-color: var(--vscode-keybindingLabel-bottomBorder);
        box-shadow: inset 0 -1px 0 var(--vscode-widget-shadow);
        vertical-align: middle;
        padding: 1px 3px;
    }

    ::-webkit-scrollbar {
        width: 10px;
        height: 10px;
    }

    ::-webkit-scrollbar-corner {
        background-color: var(--vscode-editor-background);
    }

    ::-webkit-scrollbar-thumb {
        background-color: var(--vscode-scrollbarSlider-background);
    }
    ::-webkit-scrollbar-thumb:hover {
        background-color: var(--vscode-scrollbarSlider-hoverBackground);
    }
    ::-webkit-scrollbar-thumb:active {
        background-color: var(--vscode-scrollbarSlider-activeBackground);
    }
    ::highlight(find-highlight) {
        background-color: var(--vscode-editor-findMatchHighlightBackground);
    }
    ::highlight(current-find-highlight) {
        background-color: var(--vscode-editor-findMatchBackground);
    }
</style>
bendera commented 1 month ago
  1. Fixed in the pre-release:

npm i @vscode-elements/elements@next

veredcon commented 1 month ago

I have an issue migrating the vscode-single select and I don't understand why, I tried many things we have there 2 dropdowns that needs to be populated with the selected org and space once they appear.. for some reason I get the selected org as expected (option is selected) but not the selected space (the selected space is correct but not rendered in the spaces dropdown)..

Could you please attach a minimalistic reproducible example?

Sure, thanks Adam!

  1. Clone https://github.com/SAP/cloud-foundry-tools.git
  2. git checkout toolkitmig - this branch contains migration of webview toolkit to this lib. The last commit I added only to reproduce this issue so you will not need to do actual login to Cloud Foundry.
  3. yarn, yarn format:fix, yarn ci
  4. Click on Run Extension
  5. Open command palette and choose CF: Login to Cloud Foundry image

The issue is that when the dropdowns are displayed, they should already be selected on the relevant org and space. For some reason the space is not being preselected as expected.. (it should be space4 and not space1) image

It has worked with the former vscode-dropdown so I'm trying to understand what am I missing here ...

veredcon commented 1 month ago

By the way in order to maintain the id of the codicon css after the vite build I have added the following to the vite.config.js

{
      name: "html-transform",
      transformIndexHtml(html) {
        // Add id="vscode-codicon-stylesheet" to the existing link tag
        return html.replace(/<link rel="stylesheet" href="\.\/assets\/index-[^"]+\.css">/, (match) =>
          match.replace("<link", '<link id="vscode-codicon-stylesheet"')
        );
      },
    },
bendera commented 1 month ago

I fixed many bugs, please install the latest pre-release version from the next channel. I created a minimalistic, reproducible example: elements-vue-bug.zip

The essential part is this:

<script>
export default {
  name: "HelloWorld",
  data() {
    return {
      orgs: [],
      selectedOrg: '',
    }
  },
  methods: {
    updateSelectedOrg(event) {
      this.selectedOrg = event.target.value;
    },
  },
  mounted() {
    setTimeout(() => {
      const orgs = [
        { guid: 'A', label: 'Lorem' },
        { guid: 'B', label: 'Ipsum' },
        { guid: 'C', label: 'Dolor' }
      ];

      this.orgs = orgs;
      this.selectedOrg = orgs[2].guid;
    }, 1000);
  }
}
</script>

<template>
  <vscode-single-select :value="selectedOrg" @change="updateSelectedOrg">
    <vscode-option v-for="org in orgs" :key="org.guid" :value="org.guid" :selected="org.guid === selectedOrg">{{
      org.label
      }}</vscode-option>
  </vscode-single-select>
  <p>{{ selectedOrg }}</p>
  <select @change="selectedOrg = $event.target.value" :value="selectedOrg">
    <option :value="org.guid" v-for="org in this.orgs" :key="org.guid" :selected="org.guid === selectedOrg">{{
      org.label
    }} ({{ org.guid }})</option>
  </select>
</template>
this.orgs = orgs;
this.selectedOrg = orgs[2].guid;

This two lines populate the select component with the selectable options and set the selected one. However, in Vue (and every component based framework) the state changes will be collected and the component will be re-rendered in a single step. Because of this, the value of the select component will be set before it will be populated with the available options. So in this case setting the value won't work, but applying the "selected" property on the vscode-option will be fine.

bendera commented 1 month ago

(The second select is just for comparison.)

veredcon commented 1 month ago

Thanks a lot @bendera ! Now with the next version it works as expected! πŸ˜€πŸ™ I really appreciate addressing this issue and taking the time to solve it! We are expecting to migrate the rest of our repositories soon.