-
Notifications
You must be signed in to change notification settings - Fork 105
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
Add tests for LWC #2299
Add tests for LWC #2299
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@rictic @justinfagnani this PR is ready for review |
Friendly ping @rictic @justinfagnani – any chance of getting LWC on Custom Elements Everywhere? 🙇 |
Is this getting merged? I would love to reference the LWC results that are publicly available. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
libraries/lwc/meta/issues.json
Outdated
@@ -0,0 +1,7 @@ | |||
[ | |||
{ | |||
"link": "https://github.com/salesforce/lwc/issues/1904", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This issue is closed now. Should it be salesforce/lwc#3335 ?
libraries/lwc/meta/summary.md
Outdated
|
||
<h4 id="lwc-handling-events">Handling events</h4> | ||
|
||
LWC has no restrictions on event names when listening for events imperatively via the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No library has restrictions on addEventListener()
since it's a native method. Do other libraries/frameworks mention this this way?
Thanks for the review @justinfagnani! The PR mentions that it has workflows awaiting approval by a maintainer. Is this something you could help us with? |
Everything looks good in my local build but please let me know if I've missed anything.
I can open a PR against #2263 with the required changes after this is merged.