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

Web Components: Add Event Handling Caveat #1890

Closed
wants to merge 2 commits into from

Conversation

aztalbot
Copy link

@aztalbot aztalbot commented Aug 5, 2022

Description of Problem

The custom elements section currently includes an incorrect score from Custom Elements Everywhere. Even if Custom Elements Everywhere decided to accept custom directives as satisfactory (which they have not), I don't think the score is all that relevant. Bottom line: custom elements work, and there are some caveats.

The caveat around listening for events with capital letters currently is not documented, and neither is the recommended solution.

Proposed Solution

  • Remove the sentence referring to Custom Elements Everywhere
  • Document the caveat around listening to events with names other than lowercase and kebab-case names
  • Document the recommended solution of using a custom directive

Additional Information

Related issue: vuejs/core#5401
Related closed PR: #1553
Related RFC discussion: vuejs/rfcs#451
Closes #1708

@netlify
Copy link

netlify bot commented Aug 5, 2022

Deploy Preview for vuejs ready!

Name Link
🔨 Latest commit 3f5592f
🔍 Latest deploy log https://app.netlify.com/sites/vuejs/deploys/630ce023fdca9d0008decaff
😎 Deploy Preview https://deploy-preview-1890--vuejs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@aztalbot
Copy link
Author

@LinusBorg I see you were assigned to review. Any thoughts?

Copy link
Contributor

@skirtles-code skirtles-code left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Sorry for the delay getting back to you.

src/guide/extras/web-components.md Outdated Show resolved Hide resolved
Co-authored-by: skirtle <65301168+skirtles-code@users.noreply.github.com>
@LinusBorg
Copy link
Member

LGTM

@yyx990803
Copy link
Member

Hold it for now - I believe this should be considered a bug and is fixable.

@LinusBorg
Copy link
Member

LinusBorg commented Aug 30, 2022

Hold it for now - I believe this should be considered a bug and is fixable.

@yyx990803 For camelCase it mighr be, i think i even have an open PR for that? Not sure 😅

But in Vue core we map onUpperCase props to upperCase events, so we cant real differentiate between a camelCase and PascalCase Listener.

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 this pull request may close these issues.

Vue does not perform 100% anymore at Web Components test
4 participants