vscode-elements / elements

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

Tabs with elements in the header #32

Closed mikeburgh closed 2 years ago

mikeburgh commented 2 years ago

Firstly, great components!!

I found a small issue with the tab's.

If you have other elements inside the header, eg:

...
then clicking in the tab does not change the selected tab.

I think the reason is due to the target element not having the dataset.index value (as it's the div not header element) as per the code here: https://github.com/bendera/vscode-webview-elements/blob/master/src/vscode-tabs.ts#L57

bendera commented 2 years ago

Hi,

I'm glad you like it!

It seems this hard-coded "header" selector causes this bug: https://github.com/bendera/vscode-webview-elements/blob/master/src/vscode-tabs.ts#L101

I'm going to make this selector more flexible. Anyway, could I ask you, why you use a div element instead of a header here? I would like to see your code if it's public.

mikeburgh commented 2 years ago

Code is not public yet. The div was just an example that demonstrated it in the most extreme case (header element completely filled). I actually found it by adding a badge to the tab after some text, and noticed clicking the badge prevented the switch.

The other idea I had for a fix was to change the code to search the path for the header element and then read the dataset: event.composedPath().find(this)!.dataset.index

bendera commented 2 years ago

Fixed in 0.7.1

mikeburgh commented 2 years ago

Works great!!

bendera commented 2 years ago

Hi,

I published a pre-released version (0.8.0-pre.0), it's worth a try. I've rewritten the Tabs component, it's fully WAI-ARIA compatible. If you would like to see the new examples, check out the rel-0.8.0 branch, then run npm ci and npm run docs:start

mikeburgh commented 2 years ago

NICE!

Also noticed you added badge, I was going to tidy my hacked version up and send you a PR for it.. yours is nicer.. textfield looks very handy as well!

I tried out the pre release, got an error bundling cause of the capital I in the from of the import : https://github.com/bendera/vscode-webview-elements/blob/rel-0.8.0/src/vscode-tabs.ts#L8

mikeburgh commented 2 years ago

One thing I noticed on the new tabs.. you have text-overflow: ellipsis; set, but it does not seem to work and the tab just gets wider with long text.

I added it by placing the text inside the header in it's own div, and setting text-overflow and max-width on it:

<div>${title}</div>
<vscode-badge variant="counter">${this.badge}</vscode-badge>
div {
  display: inline-block;
  max-width: 200px;
  white-space: nowrap;
  overflow: hidden;
  text-overflow: ellipsis;
}

but that breaks up the alignment with the badge a little (which I can fix)

Screenshot 2022-11-07 at 7 23 14 PM
mikeburgh commented 2 years ago

Sorry, also just found.. it looks like you are missing the call to this._setActiveTab(); inside _onHeaderSlotChange.. so when you set the selected index outside, it does not get picked up when items are added as it did before.

bendera commented 2 years ago

Thanks for the feedback!

I've fixed the text-overflow issue. Just set the max-width on the tab-header.

when you set the selected index outside, it does not get picked up when items are added as it did before.

I could reproduce it. This happens when the [slot=header] attribute is missing. Since it is easy to fix and the public API would be cleaner, I changed the code to handle this case.

0.8.0-pre.1 has been bumped.

mikeburgh commented 2 years ago

The import of uniqueId still has the wrong case, but other changes are working great!

I think I will still need to do my own text trimming in a sub node in the tab for just the text, otherwise while the current way trims it, it also hides the badge/close icons. All good I can sort that.

Appreciate the updates.

bendera commented 2 years ago

The import path is fixed, a new release has been published.

mikeburgh commented 2 years ago

Working great now!