Add renderInto function to allow Lit templates to adopt existing DOM elements. [devtools/devtools-frontend : main]
Danil Somsikov (Gerrit)
New activity on the change![]( "Open in Gerrit")
Related details
- Philip Pfaffe
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Danil Somsikov (Gerrit)
Danil Somsikov voted![]( "Open in Gerrit")
| Auto-Submit | +1 |
| Commit-Queue | +1 |
Related details
- Philip Pfaffe
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Pfaffe (Gerrit)
Philip Pfaffe added 2 comments![]( "Open in Gerrit")
I understand the implementation, but this seems extremly complicated and risky. Why do we need this?
const boundElement = (part as any).element as HTMLElement;Do we need the any cast? Risky
Related details
- Danil Somsikov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Danil Somsikov (Gerrit)
Danil Somsikov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Danil Somsikov
| Auto-Submit | +1 |
I understand the implementation, but this seems extremly complicated and risky. Why do we need this?
PTAL, I significantly reworked it so that it supports parent content and works properly with select.
We need it, because we need an declarative way to update existing elements. Specifying jslog and classes in the Widget constructors let's us avoid the linter but doesn't still violates the separation of concerns. It is complex, but so is lit. Regarding riskiness, this is extensively tested, and I can add more if this will help address this concern.
I'm not married to this solution, if you have a practical simpler alternative solution to this problem, I'd be happy to consider it.
Related details
- Philip Pfaffe
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Pfaffe (Gerrit)
Philip Pfaffe added 1 comment![]( "Open in Gerrit")
Danil SomsikovI understand the implementation, but this seems extremly complicated and risky. Why do we need this?
PTAL, I significantly reworked it so that it supports parent content and works properly with select.
We need it, because we need an declarative way to update existing elements. Specifying jslog and classes in the Widget constructors let's us avoid the linter but doesn't still violates the separation of concerns. It is complex, but so is lit. Regarding riskiness, this is extensively tested, and I can add more if this will help address this concern.
I'm not married to this solution, if you have a practical simpler alternative solution to this problem, I'd be happy to consider it.
The separation of concerns being that we have view-logic in the presenter constructor? Aren't there cheaper solutions for moving that over? Like having a utility that sets these on the target in a view function? We could wrap lit's render with a version that takes a widgetOptions object for example
Related details
- Danil Somsikov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Danil Somsikov (Gerrit)
Danil Somsikov voted and added 2 comments![]( "Open in Gerrit")
Votes added by Danil Somsikov
| Auto-Submit | +1 |
Danil SomsikovI understand the implementation, but this seems extremly complicated and risky. Why do we need this?
Philip PfaffePTAL, I significantly reworked it so that it supports parent content and works properly with select.
We need it, because we need an declarative way to update existing elements. Specifying jslog and classes in the Widget constructors let's us avoid the linter but doesn't still violates the separation of concerns. It is complex, but so is lit. Regarding riskiness, this is extensively tested, and I can add more if this will help address this concern.
I'm not married to this solution, if you have a practical simpler alternative solution to this problem, I'd be happy to consider it.
The separation of concerns being that we have view-logic in the presenter constructor? Aren't there cheaper solutions for moving that over? Like having a utility that sets these on the target in a view function? We could wrap lit's render with a version that takes a widgetOptions object for example
As discussed, implemented as extra options for the `render` function intead.
const boundElement = (part as any).element as HTMLElement;Do we need the any cast? Risky
- Philip Pfaffe
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |