palantir / blueprint

A React-based UI toolkit for the web
https://blueprintjs.com/
Apache License 2.0
20.75k stars 2.18k forks source link

MultiSelect wipes out user defined `tagInputProps.onAdd` despite being allow in the type definition #7094

Open evansjohnson opened 3 days ago

evansjohnson commented 3 days ago

Tagging this as a bug for now but could see it being addressed with a feature to better support adding new tags on paste in the multi select, or documenting existing behavior more correctly.

Environment

Code Sandbox

https://codesandbox.io/p/devbox/exciting-snowflake-cjnlnj?file=%2Fsrc%2FCoreExample.tsx%3A30%2C1&workspaceId=5e6d7c89-89f8-459c-baa9-d2bd96e632db

Steps to reproduce

Paste a string with tabs into each input (copy paste from an Excel or other data table), observe that onAdd is not called for the MultiSelect on top. On the bottom I show a workaround with using onPaste on a wrapping div, but this also runs into the issue I opened here: https://github.com/palantir/blueprint/issues/7090

Actual behavior

User defined onAdd isn't called

Expected behavior

User defined onAdd should be called, or docs should indicate that this prop is not available. The MultiSelect onItemsPaste prop does not work for me because it always uses the itemPredicate prop which only considers available items. In my case I am asynchronously loading items based on the current query, but on paste want to simply add all pasted items, possibly creating new items.

I was eventually able to get this working by hooking up my own onPaste handler on a div wrapping the multi select since that event bubbles up. I am also separating on tab characters though and want the input to clear after paste, and eventually figured out I could do this by passing my separator to the tagInputProps even though I wasn't handling the pasting through those props.

Possible solution

See https://github.com/palantir/blueprint/blob/develop/packages/select/src/components/multi-select/multiSelect.tsx#L410

The MultiSelect component implements it's own onAdd handler for the underlying TagInput without calling one passed in by the consumer.

We should either allow the MultiSelect consumer to define onAdd, or document this as a prop that is not passed along like we do for some of the other tag input props.