-
Notifications
You must be signed in to change notification settings - Fork 881
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
Fix noted TODOs #2072
Fix noted TODOs #2072
Conversation
* Fixes doc link TODOs in LitElement and ReactiveElement * Fixes styling tests to include static bindings in lit-html and LitElement * Removes `@queryAssignedNodes` shim of `Element.matches` since this is now addressed via polyfills.
🦋 Changeset detectedLatest commit: 7d80a64 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📊 Tachometer Benchmark ResultsSummarynop-update
render
update
update-reflect
Resultslit-element-list
render
update
update-reflect
lit-html-kitchen-sink
render
update
nop-update
lit-html-repeat
render
update
lit-html-template-heavy
render
update
reactive-element-list
render
update
update-reflect
|
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.
Failing test on matches, not sure what's up since it seems like webcomponents/polyfills#400 was merged long ago.
Also cc: @arthurevans @aomarks since this is the first time we're putting website URLs in code (references to docs from devmode warnings). It would be really nice to indirect through a URL shortener so we could update links if they change. Would it be easy to make a redirect config on lit.dev so the code could link to lit.dev/msg/0001
or lit.dev/msg/method-removed
and have that redirect to a configurable spot?
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
const ElementProto = Element.prototype as any; | ||
const legacyMatches = | ||
ElementProto.msMatchesSelector || ElementProto.webkitMatchesSelector; |
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.
tests: ❌ @queryAssignedNodes > returns assignedNodes for slot that contains text nodes filtered by selector when Element.matches does not exist
tests: TypeError: node.matches is not a function
tests: at ../reactive-element/src/decorators/query-assigned-nodes.ts:60:32
tests: at Array.filter (<anonymous>)
tests: at HTMLElement.get [as footerAssignedItems] (../reactive-element/src/decorators/query-assigned-nodes.ts:57:24)
tests: at o.<anonymous> (../reactive-element/src/test/decorators/queryAssignedNodes_test.ts:221:40)
😭
Also, changeset please |
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.
I added #2078 to track whether we want to change deep lit.dev links to a flat/shortened redirectable scheme.
@queryAssignedNodes
shim ofElement.matches
since this is now addressed via polyfills.