mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.94k stars 32.27k forks source link

[Chip] Rename onDelete and deleteIcon #16097

Open MrXyfir opened 5 years ago

MrXyfir commented 5 years ago

Even the docs show onDelete and deleteIcon used for purposes other than deleting the Chip. I'd imagine it's common to use them for other purposes.

I propose renaming them to onSecondaryAction and secondaryIcon, or something similar. The current attributes could be aliases for those so as to not be a breaking change, at least for v4.

oliviertassinari commented 5 years ago

@MrXyfir Do you have an example of use case?

MrXyfir commented 5 years ago

@oliviertassinari Sure, so for example, how I'm currently using it:

A Chip represents data that can be acted upon, but in my case there are multiple possible actions, which I indicate through the primary icon. The default action is determined automatically, but the user can optionally click the secondary action to "explode" the chip into multiple others, each chip representing a different action on that original piece of data.

That probably makes no sense. Here's a hypothetical instead:

The Chip represents a tag you attach to another object in your app for organizational purposes. Clicking the tag adds or removes it. Clicking the secondary action views all objects linked to it, or maybe stars the tag so it shows first in the list, or opens a modal displaying metadata about it.

oliviertassinari commented 5 years ago

I have added the v5 milestone, at least so we don't forget about considering your proposal.

alewis729 commented 4 years ago

In the docs it's mentioned that

When the Chip is focused [...] releasing (keyup event) Backspace or Delete will call the onDelete handler [...]

In my opinion, those specific keyup events with the onSecondaryAction prop name change should be discarded, It doesn't make much sense to me to keep them.

I'd like to work on this issue btw. @oliviertassinari

oliviertassinari commented 4 years ago

@alewis729 interesting, maybe we need an onDelete prop after all, it behaves differently to a regular secondary action.