ecomfe / vue-echarts

Vue.js component for Apache ECharts™.
https://vue-echarts.dev
MIT License
9.43k stars 1.48k forks source link

feat: support native event listeners in Vue 3 #765

Closed kingyue737 closed 4 months ago

kingyue737 commented 4 months ago

fix #764

Feel free to close this PR if it is not suitable. I found useElementHover fits my usage scenario perfectly. Currently, I only saw one user who needs this feature https://github.com/ecomfe/vue-echarts/issues/516#issuecomment-1277695717

While it's trivial to add a wrapping element, I think it may be meaningful to add support for native events to ease the gap between Vue 2 and 3.

codesandbox[bot] commented 4 months ago

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders
Open Preview

vercel[bot] commented 4 months ago

@kingyue737 is attempting to deploy a commit to the EFE Team on Vercel.

A member of the Team first needs to authorize it.

Justineo commented 4 months ago

Thanks for this!

We're using the same namespace for both ECharts events and native DOM events in this PR, like so:

<v-chart @click="..." @mouseenter="..."/>

Here, we're binding a chart event alongside a native event, but it's not clear from the code which is which. This could lead to confusion, especially if ECharts introduces an event that shares its name with a native DOM event.

Perhaps a better approach would be to adopt a convention for native events similar to how we handle ZRender events, using a zr: prefix. This would help avoid any naming conflicts. This also spares us the need to list all event names already included in ECharts and ZRender, offering better adaptability to future changes.

kingyue737 commented 4 months ago

Namespace is a good idea. Miss the removed .native modifier 😭. In this PR, we conform to the default behavior of Vue 3 documented, a native event sharing the same name with echarts/zrender will be ignored.

Vue 3 documentation: If a native event (e.g., click) is defined in the emits option, the listener will now only listen to component-emitted click events and no longer respond to native click events.

This also spares us the need to list all event names already included in ECharts and ZRender, offering better adaptability to future changes.

Yeah, but we still need to list all event names like what we currently do in this repo to provide type check and auto-completion in SFC template. However, this PR moves event names from types to JavaScript code, forcing this repo to maintain close updates to keep up with echarts.

Since I don't need this feature currently, it might be good to maintain the status quo XD

Justineo commented 4 months ago

In this PR, we conform to the default behavior of Vue 3 documented, a native event sharing the same name with echarts/zrender will be ignored.

This leads to breaking changes if ECharts introduces an event that shares its name with a native DOM event.

Justineo commented 4 months ago

Maybe we can do this:

<v-chart @native:click="..." @click="..." />

So that we still leave a escape hatch for those who want to use native event bindings.

kingyue737 commented 4 months ago

Good idea. Preparing for a new PR