What about something like Polymer's notify (automatic property change notification events) to make upward data flow easier?
Problem:
lit-element (lit-html) doesn't support 2-way binding. The correct way to pass data upwards is to use events.
E.g. for <paper-toggle-button> I can listen on-checked-changed, and then create a handler that sets the toggled property to the value I get from the event:
static get properties() {
return {
toggled: Boolean
}
}
_render({toggled}) {
return html`
<paper-toggle-button
checked="${toggled}"
on-checked-changed="${this.onCheckedChanged.bind(this)}">
</paper-toggle-button>
${toggled}
`;
}
onCheckedChanged(e) {
this.toggled = e.detail.value;
}
But what if I want to use <mwc-switch> (which is also based on lit-element) instead of <paper-toggle-button>?
The problem is that <mwc-switch> has no the checked-changed custom event unlike <paper-toggle-button>.
(The fact that <mwc-switch> has no the checked-changed custom event, shows that material-web-components violate Property Change Events rule of the The Gold Standard Checklist for Web Components.)
I've found https://github.com/DiiLord/lit-element and it has notify.
What about to port it to this element? It helps implement property change notification events as easy as adding notify to property in static get properties().
@sorvell @justinfagnani @azakus @kenchris PTAL.
(The fact that
has no the checked-changed custom event, shows that material-web-components violate Property Change Events rule of the The Gold Standard Checklist for Web Components.)
I would actually say: mwc-switch should start firing the event, rather than making any update to LitElement.
@TimvdLippe You are right, I mean that adding a few lines of code to lit-element to implement notify can save a bunch of code lines and time for implementation of custom property change notification events in thousands of other web components.
@FluorescentHallucinogen I think the _didRender(props, changedProps, prevProps) already can take care of that? It should provide you a full dictionary of property name to current and the previous value. You can then call it something like this (havent' run the code, but it is a gist of what I am trying to convey):
_didRender(props, changedProps, prevProps) {
if (changedProps['checked']) {
this.dispatchEvent(new CustomEvent('checked-changed', {detail: {prev: prevProps['checked'], current: this['checked'] } } };
}
}
Acknowledged this is a little bit of a boilerplate, but I think introducing the wrong abstraction in LitElement is worse off than letting users decide how they want to implement this logic.
@TimvdLippe
but I think introducing the wrong abstraction in LitElement is worse off than letting users decide how they want to implement this logic.
Why notify is wrong abstraction? Are you sure that all web component developers know how to implement this right?
The first time you do something, you just do it. The second time you do something similar, you wince at the duplication, but you do the duplicate thing anyway. The third time you do something similar, you refactor.
— from Refactoring: Improving the Design of Existing Code book.
Maybe I misunderstand the purpose of lit-element…
LitElement uses lit-html to render into the element's Shadow DOM and Polymer's PropertiesMixin to help manage element properties and attributes.
I actually have the same problem with lit-element, that none of them seem to fire any events that can be listened to by ancestors. I even tried to dispatchEvent(...) of my own, but to no avail - no events seem to leave my element and I can't find any documentation or examples that would actually work.
I also agree that having a notify: true on your property to dispatch on-Property-changed event is a good idea as it simplifies a lot of repeating code where an element needs to dispatch custom event. What are LitElements for if not to provide a simple, light-weight framework for building our own elements?
@pitus that's because some (and custom) events do not cross the shadow dom boundary by default. You need to set composed to true.
const event = new CustomEvent('my-event', { bubbles: true, composed: true });
this.dispatchEvent(event);
@pshihn - yes, I got that suggestion from @eskan already and it works, but it's NOT being used by mwc components at the moment so I can't listen to on-changed on the mwc-textfield for example, although on-input does work with the latest change to textfield (as it's already componsed according to @eskan).
However, adding a notify and observer attributes on properties might be very useful and time-saving for custom elements so I don't see why it's a bad idea to have these added to the LitElement?
We generally agree with the Property Change Events rule, but Polymer's notify function did not follow this rule since it fired events on every change so it's not appropriate to include here.
Right now we're disinclined to add any sugar over the native dispatchEvent for this type of thing since it's typically under very specific circumstances that an element needs to fire an event due to an internal change. If there's a common pattern that emerges here, we may reconsider this.
Since specific issues noted here are actually with the mwc elements, we're closing this issue in favor of https://github.com/material-components/material-components-web-components/issues/63.
Doesn't work this.dispatchEvent for me. What is the correct way to fire an event from my custom Element?
Most helpful comment
I would actually say:
mwc-switchshould start firing the event, rather than making any update to LitElement.