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

Using custom component as dropdown trigger #21

Closed
elilabes opened this issue Sep 30, 2020 · 6 comments
Closed

Using custom component as dropdown trigger #21

elilabes opened this issue Sep 30, 2020 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@elilabes
Copy link

elilabes commented Sep 30, 2020

I've started having a play around with this today and it's awesome! I ran into an issue though and I'm not sure if it is because I am doing something wrong / not using it as intended or if there is a bug.

I would like to use a custom button component to trigger the dropdown like so,

<MenuButton as="template"><MyCustomButton>Options</MyCustomButton></MenuButton>

however when I open the dropdown, I get the following error:
[Vue warn]: Property "id" was accessed during render but is not defined on instance.
Uncaught TypeError: _buttonRef$value.contains is not a function

Im using the latest version of Vue 3, 0.1.3 of Headless UI.

@RobinMalfait
Copy link
Member

By default we use a button, and we put a bunch of properties on that button. If you use a custom button (like how you did it) then we put all those properties on the custom component. If I explore this in codesandbox than you can see that all the correct stuff is applied (You can either inspect the DOM or the attrs) https://codesandbox.io/s/headlessuivue-menu-example-forked-vvzft?file=/src/App.vue

However, the actual DOM ref is not applied correctly. I have to come back to this issue because I am not 100% sure why this is happening (or rather not happening).

@RobinMalfait RobinMalfait added the bug Something isn't working label Sep 30, 2020
@elilabes
Copy link
Author

Hey @RobinMalfait - I was just wondering if there was any update on this issue? I'm currently just wrapping the component in a div so that it is not the first child of the MenuButton, but that causes the focus functionality on close to not work.

Happy to provide any more details if needed 😄

@RobinMalfait RobinMalfait self-assigned this Feb 5, 2021
@RobinMalfait
Copy link
Member

Hey! Sorry for the very long wait.
I don't know exactly what your particular example doesn't work.

I know that Vue has "special" behaviour implemented for refs and class. I think (but am not certain) that there is a limitation that the ref is not correctly forwarded to the deeply nested actual DOM node. I still have to take a deeper look if I can make that more clear or not.

That said, here is an alternative implementation that might work for you out of the box: https://codesandbox.io/s/headlessuivue-menu-example-forked-vvzft?file=/src/App.vue

@a47ae
Copy link

a47ae commented Feb 9, 2021

Hey, I have a similar error but I am not using the slot to render the custom button, but the as prop.

Example code for using a custom component

<template>
  <Menu>
    <MenuButton ref="reference" :as="as">
      Demo Button
    </MenuButton>
    
    <div ref="popper" class="w-56">
      <!-- menu content -->
    </div>
  </Menu>
</template>

<script lang="ts">
import { DemoButton } from 'my-ui-lib'
import { Menu, MenuButton } from "@headlessui/vue";

export default defineComponent({
  components: {
    Menu,
    MenuButton,
  },
  setup() {
    const reference = ref(null) as Ref<null | { el: { $el: HTMLElement } }>;
    const popper = ref(null) as Ref<null | HTMLElement>;

    onMounted(() => {
      watchEffect((onInvalidate) => {
        const popperEl = popper.value;
        const referenceEl = reference.value?.el?.$el;

        if (!(referenceEl instanceof HTMLElement)) return;
        if (!(popperEl instanceof HTMLElement)) return;

        const { destroy } = createPopper(referenceEl, popperEl, {
          placement: "bottom",
          strategy: "fixed",
          modifiers: [{ name: "offset", options: { offset: [0, 10] } }],
        });

        onInvalidate(destroy);
      });
    });

    return {
      reference,
      popper,
      as: DemoButton,
    };
  },
});
</script>

While the above approach seems to work I got two errors in the console.

[Vue warn]: Property "id" was accessed during render but is not defined on instance.

and

[Vue warn]: Non-function value encountered for default slot. Prefer function slots for better performance. 

But the id seems to be set correctly on my component and also the other props that are controlled through headlessui are passed correctly.

While digging in the debugger I think I found the reason for the error.
It appears to be this line: 'aria-labelledby': api.buttonRef.value?.id

Because in the above case, the button ref is not an instance of HTMLElement but a reference to a Vue component the id property does not exists on api.buttonRef.value. Instead it exists on api.buttonRef.value.$el.id

When changing this line, the error about accessing id disappears, but then it breaks for instances where no custom component is used as the button. So probably there needs to be some kind of check what kind of reference api.buttonRef is.

I could not find out what the reason for the second error is, but is seems to also be connected to using a custom component.

@a47ae
Copy link

a47ae commented Feb 9, 2021

I encountered another error which also seems to be caused by api.buttonRef not being an HTMLElement.

TypeError: _api$buttonRef$value.focus is not a function

which happens in for example in this line nextTick(() => api.buttonRef.value?.focus({ preventScroll: true }))

I think this also explains the not working focus described by @elilabes

RobinMalfait added a commit that referenced this issue Feb 19, 2021
There is probably a better way to do this, the idea is that we apply a
ref to the component. However by default for html components
`yourRef.value` will be the underlying DOM node. However if you pass the
ref to another component, the actual DOM node will be located at
`yourRef.value.$el`.

Fixes: #21
RobinMalfait added a commit that referenced this issue Feb 19, 2021
* add small dom utility to resolve the dom node from a ref

* use dom() to resolve underlying DOM node

There is probably a better way to do this, the idea is that we apply a
ref to the component. However by default for html components
`yourRef.value` will be the underlying DOM node. However if you pass the
ref to another component, the actual DOM node will be located at
`yourRef.value.$el`.

Fixes: #21

* update changelog
@RobinMalfait
Copy link
Member

Hey!

This should be fixed, and will be available in the next release.
You can already try it using npm install @headlessui/vue@dev or yarn add @headlessui/vue@dev.

Fixed it in #249, thanks for the extra pointers @a47ae.

Here is an example with the new (dev) version: https://codesandbox.io/s/headlessuivue-menu-example-forked-vvzft?file=/src/App.vue

RobinMalfait added a commit that referenced this issue Mar 22, 2021
* add small dom utility to resolve the dom node from a ref

* use dom() to resolve underlying DOM node

There is probably a better way to do this, the idea is that we apply a
ref to the component. However by default for html components
`yourRef.value` will be the underlying DOM node. However if you pass the
ref to another component, the actual DOM node will be located at
`yourRef.value.$el`.

Fixes: #21

* update changelog
RobinMalfait added a commit that referenced this issue Mar 26, 2021
* add small dom utility to resolve the dom node from a ref

* use dom() to resolve underlying DOM node

There is probably a better way to do this, the idea is that we apply a
ref to the component. However by default for html components
`yourRef.value` will be the underlying DOM node. However if you pass the
ref to another component, the actual DOM node will be located at
`yourRef.value.$el`.

Fixes: #21

* update changelog
RobinMalfait added a commit that referenced this issue Apr 2, 2021
* add small dom utility to resolve the dom node from a ref

* use dom() to resolve underlying DOM node

There is probably a better way to do this, the idea is that we apply a
ref to the component. However by default for html components
`yourRef.value` will be the underlying DOM node. However if you pass the
ref to another component, the actual DOM node will be located at
`yourRef.value.$el`.

Fixes: #21

* update changelog
RobinMalfait added a commit that referenced this issue Apr 2, 2021
* add small dom utility to resolve the dom node from a ref

* use dom() to resolve underlying DOM node

There is probably a better way to do this, the idea is that we apply a
ref to the component. However by default for html components
`yourRef.value` will be the underlying DOM node. However if you pass the
ref to another component, the actual DOM node will be located at
`yourRef.value.$el`.

Fixes: #21

* update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants