Open ZempTime opened 4 years ago
I feel as though we should provide some guidance how/where to specify and use events.
In General:
composed: false
. This prevents extra computation, keeps clean boundaries between various document subtrees.e.target
to get the info they need. Adding extra info to the event might seem convenient at first, but it expands the api surface of the component. If you need to add extra information to the event, do so sparingly.Should I prefix with chameleon
?
Initially, we thought this would be a great way to delineate where events were coming from (bootstrap does this). However, it doesn't end up delivering any extra convenience. If you're pulling in a component for use, you know what it is. And if you're trying to figure out where an event came from, event.target
has you covered. Let's save some keystrokes and drop the prefixes.
What if my event is like a browser event?
Examples here are input
, blur
, change
, etc. If your custom element is dispatching an event to replicate existing browser events, use the same name. In general, events like this are the exception to the "don't use composed
" guidance.
Is my event letting others places know something happened?
AKA, does your element have local state? If so, you're going to want to use a ${property}-${changed}
-style format.
[rfc note: if a component can have local state, then they're not strictly unidirectional, and we shouldn't advertise this component library that way. Maybe, "conveniently" unidirectional]
For example, it makes complete sense for chameleon-accordion
to maintain whether or not it's expanded
. This means, when the expanded
property changes, an expanded-changed
event is appropriate. Note, we can reuse the tagname convention here: the presence of a -
in an event name implies it's a custom element's event. This is clear, and conveys a lot of information in a small string. We can also assume that if someone is interacting with this event, they know what component it's relevant to. Therefore, keeping the property name in the event communicates useful information to the developer.
Is my event a part of the imperative interface of my component? Phrased slightly differently, what if I want to respond to events by calling methods that modify component state?
For example, the sheet listens for a close-sheet
event which, you guessed it, closes the sheet. Here, you're going to want to name the event imperatively (current- or future-tensed, as though it were a command) and make sure to include a -
. Generally, if a simple phrase describes the action your component should take when this event is received, name it that.
Should I send more than one kind of event for different states?
If the listener can interrogate the event target to find that information (imagine an accordion-toggled
event), no.
If it really simplifies or streamlines implementation, yes.
I agree 100% with dropping prefixes, especially given that most of the events that we are capturing live within the same component as the import, so you’re right, it should be pretty clear where that event came from (not to mention, <chameleon-input @chameleon.input.input>
feels a little redundant 😛
Something worth noting here is that a lot of our form elements, we actually can just rely on their underlying native events (this might require some research follow up to see if the native events are composed or not). On that note too, I do think we should be very explicit when and when not to use composed events. Do you know of any prior art/literature on that?
I also think there should be more clear guidance on imperative events and the verbiage we use around it. I prefer past tense actions like sheet-closed
given that technically we are dispatching these events after they happen. I’m not sure how we would specify which events are “imperative” though.
Re: existing cowpaths. No, I'm unaware of any.
In the case of close-sheet
, it's a way of exposing the interface of the component (the method that closes the sheet) to components down the dom tree, as they won't have the ability to get or maintain a reference to sheet
to call the toggle()
method directly. This is important because since it can accept arbitrary slotted content, it could get arbitrarily deep. This let's us expose the ability for content inside the sheet to close it (as we do with the close icon in sheet-content
). This inversion of control is important and grants a lot of flexibility.
So this is a case where we're actively, trying to expose a method in an imperative fashion, and our way of doing that is through an event
(it's the reverse of passing properties down). When we want an element to modify it's internal state in response to it's descendants, I think it makes sense to dispatch an imperatively-named event which says "do this."
I think relying on underlying native elements is the right way to go, although these will themselves contain uncontrolled state.
Given everything that’s been mentioned here, I’m going to add a draft write up to the wiki (mostly just going to copy what you’ve written here) and we can ratify it once we feel that everyone has had a sufficient time to provide their feedback.
Here's a brief runthrough of current events, and their future state (if any obvious ones according to these guidelines). I think we're going to need to apply some thought to some of these. | place | old | new |
---|---|---|---|
packages/accordions/src/ChameleonAccordion.js#68: | chameleon.accordions.expanded-changed |
expanded-changed |
|
packages/tabs/src/ChameleonTab.js#19: | chameleon.tabs.selected-changed |
selected-changed |
|
packages/timezone/src/ChameleonTimezone.js#148: | chameleon.timezone.input |
change , input |
|
packages/input/src/ChameleonInput.js#285 | chameleon.input.input |
change , input |
|
packages/chip/src/ChameleonChip.js#33: | remove-chip |
remove-chip |
|
packages/toast/src/ChameleonToast.js#55 | close-toast |
close-toast |
|
packages/dialog/src/ChameleonDialog.js#92: | toggle-dialog |
close-overlay |
|
packages/dialog/src/ChameleonDialog.js#100 | go-back |
||
packages/date/src/ChameleonDate.js#366 | chameleon.date.input |
input |
|
packages/table/src/ChameleonTable.js#243: | chameleon.table.change |
change |
|
packages/select/src/ChameleonSelect.js#341 | chameleon.select.input |
input |
|
packages/modal/src/ChameleonModal.js#69 | toggle-modal |
close-overlay |
|
packages/paginator/src/ChameleonPaginator.js#182 | page-change |
current-page-changed |
|
packages/multiselect/src/ChameleonMultiselect.js#347: | chameleon.select |
change , input |
|
packages/multiselect/src/ChameleonMultiselect.js#360 | chameleon.search |
search |
As a point of clarity, close-overlay
is what closes the sheet. It doesn't close, and then dispatch that event. The sheet listens for and responds to the event by closing.
Problem I'm wondering if it would behoove us to:
tabselect
).What's a browserlike event? A short, all-lowercase, no delimiter imperative phrase like "click" or "mousemove". If we need to use a delimiter, we could use
-
:
.
etcRollout: This would involve two phases:
chameleon.tab-selected
turns intotabselect
, if we decide that would be better thanclick
). Events continue to work as-is.Questions & Research: