material-web
material-web copied to clipboard
md-filter-chip: remove event and its usability
Description
Hi, first of all, thanks a lot for fixing #4951 . During the last few days I have been upgrading my client websites' with the new material components and today I stumpled upon this scenario with <md-filter-chip> (and <md-input-chip> as well).
Right now the documentation does not mention any event at all which is not really helpful if you want to know when a chip get removed. I see it can be fixed with additional annotation:
* @fires remove Fired once the chip is removed
However, another issue is when remove is fired, the actual chip is not removed yet. This causes very awkward code when handling the event where you have to consider that possibility, for example, when getting values from the children, you have to keep a reference to the chip that is being removed:
#onChipRemoved() {
// The removed chip is not removed yet at this time
if (this.#lstSplitTimes.children.length <= 1) {
this.#lstSplitTimes.append(this.#lblNoSplit);
}
// Code for getting values from children is impossible here without keeping a reference to the
// chip that fired this event
}
I understand you should not remove it yet because the user can cancel it using preventDefault. My suggestion is to give us another event, like removed so we can simply handle it if we don't care about cancelling:
function handleRemoveClick(this: Chip, event: Event) {
if (this.disabled) {
return;
}
event.stopPropagation();
const preventDefault =
!this.dispatchEvent(new Event('remove', {cancelable: true}));
if (preventDefault) {
return;
}
this.remove();
// Add this:
this.dispatchEvent(new Event('removed'));
}
P.s. I know it's discussed before but I still wish the events bubble, or at least re-dispatched by the <md-chip-set> so I don't have to add event handlers to each <md-...-chip> 😅
Browser/OS Environment
No response
I think it's the same problem as https://github.com/material-components/material-web/issues/4951 before.
We could remove the chip right on the click event, and revert it if the user explicitly used preventDefault by attaching the chip back where it was located, that would fix the issue.
But we are shifting the problem to users that want to prevent the default because now they would see a removed chip even tough they prevented the removal.
So I agree having a custom event e.g. removed would be way more convenient. It's not just for chips, I also faced this issue with checkboxes and switches.
A temporary solution you may be aware of is waiting the update to complete,
async #onChipRemoved() {
await Promise.all(this.chipset.chips.map(c => c.updateComplete));
// Changes are now reflected
// ...
}
But I understand your concern.
I think this makes sense, but we probably won't introduce it until we add chip removal animations.
The chip set is supposed to listen to the remove request, then animating the chip removing. I think the chip set should dispatch events when chips are removed or added.
@asyncLiz that's great. When you design and implement it please consider how you will dispatch it from the chip set (I assume you won't give individual chip event anymore) so we still have information of which chip is getting removed (are you providing the info through a CustomEvent.detail, what will you provide: an index, its value or the entire Element reference).