Move defaultEventNames into Schema#662
Conversation
This allows applications to extend the mapping with support for custom elements. Closes hotwired#660
lb-
left a comment
There was a problem hiding this comment.
Just a few thoughts, I think this would be an awesome feature and helps for mixed adoption of Web Components.
| // [0-9] | ||
| ...objectFromEntries("0123456789".split("").map((n) => [n, n])), | ||
| }, | ||
| defaultEventNames: { |
There was a problem hiding this comment.
| defaultEventNames: { | |
| defaultEventMappings: { |
Since we use the term mappings already, might be nice to align with that naming?
There was a problem hiding this comment.
I ended up dropping 2ddac5f so this is relevant again. I'm open to renaming it.
|
This probably needs docs also |
Co-authored-by: LB (Ben Johnston) <mail@lb.ee>
Definitely up for adding docs. Can I interpret your review as tacit support for this implementation vs. #661? |
|
I'm not on the core team - but I love the idea of this feature so it gets my vote 👍 |
2ddac5f to
0af60e5
Compare
0af60e5 to
4ecb43d
Compare
|
This now takes into account the feedback on #661 as well, setting on an I opted for the plural form of the name and passing an object because it felt nicer to write this: app.registerDefaultEventNames({
'sl-button': 'click',
'sl-select': 'change',
'sl-input': 'input',
// etc.
})vs this: app.registerDefaultEventName('sl-button', 'click')
app.registerDefaultEventName('sl-select', 'change')
app.registerdDefaultEventName('sl-input', 'click')
// etc.... but I'm open to changing it back to Other concernsThere's a limitation (also covered in the docs) where this API won't work when calling Application.start(document.body, {
...defaultSchema,
defaultEventNames: {
...defaultSchema.defaultEventNames,
'sl-button': 'click',
// etc.
}
}) |
This allows applications to extend the mapping with support for custom elements.
Closes #660
See also: this comment #661 (comment)