Skip to content

Multi-select Dropdown does not clear value after "onAddItem" #1952

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

Closed
lottamus opened this issue Aug 10, 2017 · 7 comments · Fixed by #1956
Closed

Multi-select Dropdown does not clear value after "onAddItem" #1952

lottamus opened this issue Aug 10, 2017 · 7 comments · Fixed by #1956
Labels

Comments

@lottamus
Copy link
Contributor

lottamus commented Aug 10, 2017

Steps

  1. Type a new value into the input field such as "foo"
  2. Click "Add foo" to add the new item

Expected Result

Input field will have a label with value "foo" and current typed value will be empty

Actual Result

Input field has a label with value "foo" and current typed value is still "foo"

Version

Breaks on 0.71.3+

Looks like it may have come from this commit: 62507c2#diff-609214949026e594dbfd98481664c928R705

Testcase

https://codepen.io/lott/pen/PKjQge

@levithomason
Copy link
Member

Ah, yep. We should certainly clear the query an item is added. Easy fix.

@rushabhy
Copy link

Can I try to fix this?

@levithomason
Copy link
Member

@rushabhy absolutely! I'd suggest opening the PR as early as possible, just ping me when it is ready for review.

@levithomason
Copy link
Member

Apologies, I've spoken too soon. It appears #1956 has already been opened.

@rijk
Copy link
Member

rijk commented Aug 20, 2017

I just tested, and unfortunately this does not work when calling makeSelectedItemActive( e ) on the Dropdown manually. This used to work without a problem. Now, I have to call clearSearchQuery() as well, and with a setTimeout otherwise it doesn't work. This results in a flash where first the value is added, and then the input field is cleared (instead of both happening at once).

To provide some context: I'm trying to build a typical email "to" input, where you can type multiple email addresses. The manual call is for adding on space or comma as well. Here is the code:

// Add on space to mirror email programs
if ( value.substr( -1 ) === ' ' || value.substr( -1 ) === ',' ) {
	this.refs.input.makeSelectedItemActive( e )
	setTimeout( () => this.refs.input.clearSearchQuery(), 1 )
}

@levithomason
Copy link
Member

Component instance methods are private APIs. A ref should not be used to call a child's method. In idiomatic React, only the props API should be used to interact with a component.

Instead, you should hook into the provided callbacks and update the provided props to achieve the same result. If you give a little more detail or a codepen (fork this one), I may be able to help guide you on how to do this in your case.

@rijk
Copy link
Member

rijk commented Aug 20, 2017

Hi Levi, thanks, got you. The thing is, I have to hook into the internal ones to do what I want to.

The behaviour I want to create is exactly the same as the "to" field of an email app like OSX Mail (or https://wetransfer.com).

  • Type an email address
  • Add on enter, space, comma or semicolon
  • Navigate list of recipients with mouse click or arrows, backspace to delete selected item (this does not work today)
  • Only show dropdown menu when there are options — never show the "add xxx" item (not possible today, did a CSS hack to just hide the dropdown menu altogether)
  • Cancel add but preserve value (while triggering the error property on the Dropdown) when the input was not a valid email address (for this I had to implement a custom handleAddItem implementation, see below)
handleAddItem( e, { value } ) {
  const emailRegex = /^[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/
  if ( !emailRegex.test( value ) ) {
    this.setState({ error: value })
    setTimeout( () => {
      this.refs.input.setState({ searchQuery: value })
      this.refs.input.searchRef.focus()
    }, 10 )
    return false
  }

  this.setState({ options: [ ...this.state.options,
    { value: value, text: value }
  ] })
}

Here is a pen with the current version of my component: https://codepen.io/anon/pen/PKRJEL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants