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: Update events binding in Vue render #19860

Merged
merged 4 commits into from
Jan 18, 2023

Conversation

gitstart
Copy link
Contributor

Issue: #19318

What I did

  • Vue2 renderer renders only v-bind="$props" in component template which missed the right way of binding custom events in Vue 2 with v-on or @..., in this PR, we base on argType.table.category events to add events binding into template.

How to test

cd code
yarn task --task dev --template vue-cli/vue2-default-js 
open http://localhost:6006/?path=/story/lib-store-hooks--use-state

Open Interactions panel and verify all tests are passed

image

@ndelangen ndelangen assigned ndelangen and unassigned shilman Jan 13, 2023
@ndelangen ndelangen requested review from a team and pksunkara January 13, 2023 23:05
@ndelangen
Copy link
Member

@storybookjs/vue is this good to merge?

@Aaron-Pool
Copy link
Contributor

@storybookjs/vue is this good to merge?

Looks fine to me, though the cleanest approach would be to also pull the properties that correspond to events off the $props object that's getting passed to v-bind so that duplicate, unintended attributes don't get passed along, potentially gumming up the works in lower down components.

@ndelangen
Copy link
Member

@gitstart or @Aaron-Pool could either of you make that suggested change? 🙏

@ndelangen ndelangen requested a review from Aaron-Pool January 14, 2023 00:33
@ndelangen
Copy link
Member

@storybookjs/vue can someone give this the 👍 in a code-review? Then I'll merge it.

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

Successfully merging this pull request may close these issues.

5 participants