Skip to content
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

style prop of ComboboxOptions is ignored #3248

Closed
jussirantala opened this issue May 28, 2024 · 6 comments · Fixed by #3250
Closed

style prop of ComboboxOptions is ignored #3248

jussirantala opened this issue May 28, 2024 · 6 comments · Fixed by #3250
Assignees

Comments

@jussirantala
Copy link

jussirantala commented May 28, 2024

What package within Headless UI are you using?

@headlessui/react

What version of that package are you using?

v2.0.4

Describe your issue

If you use style prop which exists in ComboboxOptions types, everything you set there is ignored. This makes it impossible to change z-index via props dynamically.

I think you need to make mergePropsAdvanced() to support merging object props together: https://github.com/tailwindlabs/headlessui/blob/main/packages/%40headlessui-react/src/utils/render.ts#L339

@jussirantala jussirantala changed the title style prop of portaled ComboboxOptions is ignored style prop of ComboboxOptions is ignored May 28, 2024
@RobinMalfait RobinMalfait self-assigned this May 28, 2024
@RobinMalfait
Copy link
Member

Hey!

This should be fixed by #3250, and will be available in the next release.

You can already try it using:

  • npm install @headlessui/react@insiders.

@jussirantala
Copy link
Author

Hey!

This should be fixed by #3250, and will be available in the next release.

You can already try it using:

  • npm install @headlessui/react@insiders.

@RobinMalfait I tried 0.0.0-insiders.f5ac361 and it still happens 🤔I tried to delete node_modules in case it's a cache issue or something and checked I have that version there.

image

I'm thinking if you instead should fix the props merging functions to support object props and merging of them. You use mergePropsAdvanced in Portal for example. Now it just replaces theirProps.style with ourProps.style instead of merging them.

@RobinMalfait
Copy link
Member

Hmm, if you are using Next.js for example, make sure to also drop the .next folder since it contains caches.

They should be merged correctly:

image image

We could merge it in the props merging logic, but this gives us a bit more control on a per component basis. We could still do that, but I think something else is going on.

@jussirantala
Copy link
Author

jussirantala commented Jun 3, 2024

Hmm, if you are using Next.js for example, make sure to also drop the .next folder since it contains caches.

They should be merged correctly:

image image
We could merge it in the props merging logic, but this gives us a bit more control on a per component basis. We could still do that, but I think something else is going on.

@RobinMalfait

We don't use next.js.

Could you try with portal=true?

@RobinMalfait
Copy link
Member

@jussirantala the anchor prop has an implicit portal={true} behind the scenes. Both with and without a portal it works as expected for me.

Can you create a minimal reproduction repo and share it here so that I can take a look?

@jussirantala
Copy link
Author

I got it working, thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants