Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Chip] Rename onDelete and deleteIcon #16097

Open
MrXyfir opened this issue Jun 6, 2019 · 5 comments
Open

[Chip] Rename onDelete and deleteIcon #16097

MrXyfir opened this issue Jun 6, 2019 · 5 comments
Labels
breaking change component: chip This is the name of the generic UI component, not the React module! discussion

Comments

@MrXyfir
Copy link

MrXyfir commented Jun 6, 2019

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
Copy link
Member

@MrXyfir Do you have an example of use case?

@oliviertassinari oliviertassinari added the component: chip This is the name of the generic UI component, not the React module! label Jun 6, 2019
@MrXyfir
Copy link
Author

MrXyfir commented Jun 6, 2019

@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 oliviertassinari added this to the v5 milestone Jun 6, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 6, 2019

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

@alewis729
Copy link

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
Copy link
Member

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

@oliviertassinari oliviertassinari removed this from the v5 milestone Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: chip This is the name of the generic UI component, not the React module! discussion
Projects
None yet
Development

No branches or pull requests

3 participants