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

Vue 3? #238

Closed
9mm opened this issue Sep 9, 2020 · 26 comments · Fixed by #563
Closed

Vue 3? #238

9mm opened this issue Sep 9, 2020 · 26 comments · Fixed by #563

Comments

@9mm
Copy link

9mm commented Sep 9, 2020

Hey, does this work with vue 3? I'm trying to use it as a simple directive and it doesn't seem to be firing the log statement. I'm not sure how exactly to debug this though

@9mm
Copy link
Author

9mm commented Sep 9, 2020

This was the code I used in its entirety in my component:

<script>
  import vClickOutside from 'v-click-outside'

  export default {
    directives: {
      clickOutside: vClickOutside.directive
    },
    methods: {
      onClickOutside (event) {
        console.log('Clicked outside. Event: ', event)
      }
    }
  };
</script>

<template>
  <div v-click-outside="onClickOutside"></div>
</template>

@ndelvalle
Copy link
Owner

ndelvalle commented Sep 10, 2020

@9mm I haven't cheked Vue 3 yet. I think some changes are needed in order for this directive to work. We should investigate a bit.

@9mm
Copy link
Author

9mm commented Sep 10, 2020

I'm new to vue and sort of this front-end JS madness in general, so I'm not exactly sure how I could help with a PR effectively. With that said I did some digging and I'm assuming the primary reason it's breaking is because vue 3 supports fragments instead of a root node, so it looks lik the bind event youre hooking into no longer exists:

https://v3.vuejs.org/guide/custom-directive.html#hook-functions

image

@9mm
Copy link
Author

9mm commented Sep 10, 2020

Later on down the page:

  • In 3.0, with fragments support, components can potentially have more than one root nodes. This creates an issue when a custom directive is used on a component with multiple root nodes.

@dnlsndr
Copy link

dnlsndr commented Sep 22, 2020

Since vue3 is now released, it would be really nice to get this working.
I'm not really all too familiar with the vue3 api so I can't really contribute to a PR though...

@renatodeleao
Copy link
Collaborator

We have now the official migrating guide.

From a quick glance, it terms of actual code it might just be a matter of updating lifecycle method names to the new ones (since the remaining api is same) and adding the disclaimer mentioned by @9mm when using the directive on custom components that can now have multiple root nodes: for now we can recommend to avoid usage in such scenarios or we would have to loop through the multiple root nodes and bind the directive to each one. At least, in theory 😛

Now in terms of this package structure, I don't how can we make it interoperable with both Vue versions (2.x and 3.x) in a single export, but maybe someone more experienced in OSS can shed a light on the best take. The most common pattern seems to be bump a major version vue-click-outside@4.x for Vue@3.x and keep a legacy branch and 3.x releases for vue@2.x.

Keen to other options, since code will practically be the same. Something that popped into my head was if we could evaluate vue version and just change export lifecycle names but i might be thinking too naively?

cc: @ndelvalle

@dnlsndr
Copy link

dnlsndr commented Sep 28, 2020

Aa far as i could make out, many libraries create a next branch, which holds the new code. The next branch will create pre-releases which can then be uploaded to npm under the @next tag. So someone can then just install v-click-outside@next with npm or yarn. As soon as vue3 becomes the primary release and vue2 is deprecated, the next brach is merged into the master branch. The old code can then be extracted into a new brach like 'vue2' or so. But not too sure if that's the best method.

@ndelvalle
Copy link
Owner

Thinking out loud, for a regular library I would certainly go with the @next version approach. But I think vue 2 will be around for a long time so having support for vue2 and vue3 in master if possible would be awesome, at least for a while.

Of course, we need to investigate a bit more, sadly I haven't been using vue for a while (Job requires me to do something else) and I was not able to read about vue 3 yet. But if its just an "export issue", we could have a default export and a vue2 version export.

So if we can reuse the code the vue2 version will require an explicit import like import { v2 } from... and vue3 will end up being the default, or maybe { v3 } not sure

@dnlsndr
Copy link

dnlsndr commented Oct 4, 2020

@ndelvalle Interesting idea! This gives me some bad smell though. Having two separate versions in the same package seems a bit overkill. Although v-click-outside ist a tiny library compared to other ones, this still doesn't seem like a clean solution, since now you are shipping two versions of the same functionality for two vue versions doubling the package src size. People might get confused as to what version they should import from the library and might deem it broken.

@Over-Easy
Copy link

Hey! I was also checking in about this, I use v-click-outside and just migrated our app to vue 3!

@dominykasgithub
Copy link

any updates?

maybe we could do something like this?

import vClickOutside from 'v-click-outside'
app.directive('observe-visibility', {
    beforeMount: (el, binding, vnode) => {
        vnode.context = binding.instance;
        vClickOutside.bind(el, binding, vnode);
    },
    update: vClickOutside.update,
    unmounted: vClickOutside.unbind,
});

@dominykasgithub
Copy link

ok, so if no package is available right now, you can use this

app.directive("click-outside", {
    beforeMount: function (el, binding) {
        // Define ourClickEventHandler
        const ourClickEventHandler = event => {
            if (!el.contains(event.target) && el !== event.target) {
                // as we are attaching an click event listern to the document (below)
                // ensure the events target is outside the element or a child of it
                binding.value(event); // before binding it
            }
        };
        // attached the handler to the element so we can remove it later easily
        el.__vueClickEventHandler__ = ourClickEventHandler;

        // attaching ourClickEventHandler to a listener on the document here
        document.addEventListener("click", ourClickEventHandler);
    },
    unmounted: function (el) {
        // Remove Event Listener
        document.removeEventListener("click", el.__vueClickEventHandler__);
    }
})

@crutch12
Copy link

@dominykasgithub could you provide PR for this? Add vue3 to dependencies and other important things?

@dominykasgithub
Copy link

@crutch12 I'm busy right now migrating from vue2, I'm way behind deadline. Maybe later if issue still persists

@9mm
Copy link
Author

9mm commented Nov 8, 2020

@dominykasgithub was your implementation identical to v-click-outside or a 'light' version which doesn't handle as many edge cases? I'm getting to point where I really need to implement this, but if your version is equal-to or better ill just use that

@Livijn
Copy link

Livijn commented Nov 11, 2020

For those of you that needs TypeScript support:

main.ts

//...
import clickOutside from './library/click-outside';

createApp(App)
  .directive('click-outside', clickOutside);
  .mount('#app');

click-outside.ts

import { DirectiveBinding } from "@vue/runtime-core";

interface ClickOutsideElement extends HTMLElement {
  __vueClickEventHandler__: any
}

export default {
  beforeMount: function (el: ClickOutsideElement, binding: DirectiveBinding) {
    const ourClickEventHandler = (event: MouseEvent) => {
      if (! el.contains(event.target as HTMLElement) && el !== event.target) {
        binding.value(event);
      }
    };

    el.__vueClickEventHandler__ = ourClickEventHandler;

    document.addEventListener("click", ourClickEventHandler);
  },
  unmounted: function (el: ClickOutsideElement) {
    document.removeEventListener("click", el.__vueClickEventHandler__);
  }
}

@jgerigmeyer
Copy link

I'm using a modified version until this issue is officially resolved: https://gist.github.com/jgerigmeyer/87d16753c93762132943a26ea40cc665

@stepanjakl
Copy link

I'm using a modified version until this issue is officially resolved: https://gist.github.com/jgerigmeyer/87d16753c93762132943a26ea40cc665

@jgerigmeyer Yay, that works well! I just had to convert it to JS (via https://www.typescriptlang.org/play)

@pbowyer
Copy link

pbowyer commented Jan 26, 2021

Thanks @jgerigmeyer.

For anyone else who like me is new to Vue, here's how I got it to work in my Vue 3 app.
In main.js:

import { createApp } from 'vue'
import App from './App.vue'
import vClickOutside from './library/v-click-outside'

let app = createApp(App)
app.directive('clickOutside', vClickOutside)
app.mount('#app')

And then in my component, I called it like:

<template>
  <div class="inline-block text-left" v-click-outside="closeDropdown">
  </div>
</template>

@andymark-by
Copy link

Temporarily created this solution for myself based on the comments above https://github.com/andymark-by/vue3-click-outside

It works fine for me

@raphaelbernhart
Copy link

Temporarily created this solution for myself based on the comments above https://github.com/andymark-by/vue3-click-outside

It works fine for me

Works fine. TS support would be very nice!

@ngekoding
Copy link

Thanks @jgerigmeyer.

For anyone else who like me is new to Vue, here's how I got it to work in my Vue 3 app.
In main.js:

import { createApp } from 'vue'
import App from './App.vue'
import vClickOutside from './library/v-click-outside'

let app = createApp(App)
app.directive('clickOutside', vClickOutside)
app.mount('#app')

And then in my component, I called it like:

<template>
  <div class="inline-block text-left" v-click-outside="closeDropdown">
  </div>
</template>

I just try it today, but getting this error.

Error

Any help?

@raphaelbernhart
Copy link

Thanks @jgerigmeyer.

For anyone else who like me is new to Vue, here's how I got it to work in my Vue 3 app.

In main.js:

import { createApp } from 'vue'

import App from './App.vue'

import vClickOutside from './library/v-click-outside'

let app = createApp(App)

app.directive('clickOutside', vClickOutside)

app.mount('#app')

And then in my component, I called it like:

I just try it today, but getting this error.

Error

Any help?

I've used the solution of andymark-by. It works fine!

@ngekoding
Copy link

@raphaelbernhart I was try it before found this issue, I got an error also.

Maybe that because I use Vite?

@ngekoding
Copy link

@raphaelbernhart I was try it before found this issue, I got an error also.

Maybe that because I use Vite?

Finally I use another package that working well with Vue 3. I use vue3-click-away.

@onurusluca
Copy link

I have been using this but I have a problem. It detects click-outside wrong on router change in a iframe for some reason. When router changes it closes the window.

So I found this. It even works with iframes and on route change. click-outside-vue3

chme added a commit to chme/forked-daapd that referenced this issue Jan 1, 2022
chme added a commit to chme/forked-daapd that referenced this issue Feb 12, 2022
ndelvalle added a commit that referenced this issue Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.