[3.x] ComboBox positioner z-index doesn't work

[codercms]

Current Behavior

The ComboBox property positionerZIndex has no effect because it's applied to the positioner element, but @zagjs/popper expects custom z-index to be applied to the first child element of the positioner:
https://github.com/chakra-ui/zag/blob/9ca9b9edd448071351271465629024ecb1eb80be/packages/utilities/popper/src/get-placement.ts#L185

https://github.com/chakra-ui/zag/blob/9ca9b9edd448071351271465629024ecb1eb80be/packages/machines/combobox/src/combobox.connect.ts#L27

skeleton/packages/skeleton-svelte/src/lib/components/Combobox/Combobox.svelte

Line 126 in b1e64c4

<div {...api.getPositionerProps()} transition:fade={{ duration: 100 }} class="{positionerBase} {positionerZIndex} {positionerClasses}">

Expected Behavior

The positionerZIndex prop should be applied to the first child element of the positioner element, not to the positioner itself

Steps To Reproduce

No response

Link to Reproduction / Stackblitz

No response

More Information

No response

[phamduylong]

I'll throw a fix for this, seems simple enough.

EDIT: maybe we need this for Modal, Popover and Tooltip as well

[endigo9740]

@phamduylong verify the information provided is correct. I'm fairly certain I tested this. Though perhaps you're right and it differs from feature to feature.

[phamduylong]

@phamduylong verify the information provided is correct. I'm fairly certain I tested this. Though perhaps you're right and it differs from feature to feature.

You might be spot-on with this one. I tested and the current version seems to work fine, the dropdown z-index seems to be applied correctly (for the combo box at least)

[endigo9740]

@codercms can you confirm what value you're passing to positionerZIndex for us please. You used shorthand in the snippet above so it was not immediately clear.

This should take any valid Tailwind z-index utility class.

For example:

<Combobox ... positionerZIndex="z-10">
	<!-- ... -->
</Combobox>

[codercms]

@endigo9740 sure, here is minimal code to reproduce this issue:

<script lang="ts">
	import { Combobox } from '@skeletonlabs/skeleton-svelte';

	const items = [
		{ label: 'Item 1', value: '1' },
		{ label: 'Item 2', value: '2' },
		{ label: 'Item 3', value: '3' },
		{ label: 'Item 4', value: '4' }
	];
</script>

<div class="max-w-72 pl-2 mt-2">
	<Combobox label="Combobox 1" data={items} openOnClick openOnKeyPress openOnChange positionerZIndex="z-10"></Combobox>
	<Combobox label="Combobox 2" data={items} openOnClick openOnKeyPress openOnChange positionerZIndex="z-10"></Combobox>
</div>

This renders to something like this:

Now lets open Combobox1 and see what happens:

As you can see the arrow from Combobox 2 overlaps Combobox 1 content:

Now let's research what styles are applied to the Combobox positioner element:
In DOM positioner element looks like:

<div data-scope="combobox" data-part="positioner" id="combobox:fnwnxmc87rk10:popper" style="position: absolute; isolation: isolate; width: var(--reference-width); top: 0px; left: 0px; transform: translate3d(var(--x), var(--y), 0); z-index: var(--z-index); --transform-origin: top center; --reference-width: 280px; --available-width: 1797px; --available-height: 497px; --x: 7.933333333333334px; --y: 83.86666666666667px; --z-index: auto;" class=" z-10  s-7L3_OeoVEYF4" data-aria-hidden="" aria-hidden="">

And here's applied styles to it:

So, we can clearly see that the provided "z-10" class was suppressed and "z-index: auto" was used instead.

Now let's change Combobox implementation a bit, we'll put positionerZIndex not to the positioner element itself, but to its first child (nav element):

<!-- Content (list) -->
<nav {...api.getContentProps()} class="{contentBase} {contentBackground} {contentSpaceY} {contentClasses} {positionerZIndex}">

Now it works correct:

And here's positioner element styles:

[endigo9740]

@codercms thanks for the information. So I think I see what's going on here. If an element has the same style set via the class attribute AND via a style attribute, the style attribute will always win out.

I've truncated your first snippet down to only the relevant parts here:

<div style="z-index: var(--z-index); --z-index: auto;" class="z-10 ">

Three things are happening:

1. Zag is expecting a --z-index CSS custom property to define the z-index value (this defaults auto)
2. Zag is setting the style attribute with this CSS custom property value.
3. We're implementing the z-10 class to the class list.

But as I mentioned above, the style attribute always takes precedence.

The interesting part here is the style attribute is set by Zag.js - which we use under the hood for components. This means they have their own mechanism for defining the z-index here. We've dealt with this for other components in the past. We actually prefer when Zag doesn't use the CSS custom property approach like this as it's kind of hard to work with.

Your solution for moving the z-index to the inner child is a work around, but I don't think that's the correct approach here. We shouldn't have two instances setting z-index like that. So we'll lean into the style attribute over the class.

Just FYI this will likely result in a breaking change as we will have to replace the prop. Moving from a string-based class, to a numeric-based value.

[endigo9740]

@phamduylong just FYI in case you were curious.

We ran into this same issue with the ProgressRing component:

• Here's where the prop is defined
• Here's where the prop value is consumed in the template

So I think what we'll do is drop the current positionerZIndex prop, then replace with something like zIndex?: number and ensure that's inserted via a style attribute like this. We'll need this for all four components.

I'm also considering reaching out upstream to Zag to see if they have some undocumented mechanism for interfacing with this.

[phamduylong]

@phamduylong just FYI in case you were curious.

We ran into this same issue with the ProgressRing component:

• Here's where the prop is defined
• Here's where the prop value is consumed in the template

So I think what we'll do is drop the current positionerZIndex prop, then replace with something like zIndex?: number and ensure that's inserted via a style attribute like this. We'll need this for all four components.

I'm also considering reaching out upstream to Zag to see if they have some undocumented mechanism for interfacing with this.

@endigo9740 I can do that as a patch, but I do think reporting this further would be advantageous still.