Skip to content

add 'onBlur' prop#44

Open
EddM wants to merge 1 commit into
instructure-react:masterfrom
EddM:add-onfocus
Open

add 'onBlur' prop#44
EddM wants to merge 1 commit into
instructure-react:masterfrom
EddM:add-onfocus

Conversation

@EddM

@EddM EddM commented May 2, 2017

Copy link
Copy Markdown

Another PR (#40) introduced an onFocus prop, which was cool, but it would be helpful if we could handle blur too!

@njj

njj commented Sep 19, 2017

Copy link
Copy Markdown

I could definitely use this too!

@michaelbrewerdavis michaelbrewerdavis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition

Comment thread src/combobox.js
* Signature:
*
* ```js
* function(userInput){}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These would not be userInput, but the focus/blur events.

@yashafromrussia yashafromrussia Oct 6, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  handleInputFocus: function() {
    this.props.onFocus();
    this.maybeShowList();
  },

This is not receiving or passing the focus event. I'm using onBlur to do error handling. So it is required for the event to be passed into this.props.onBlur(). To keep it consistent, the onFocus code should probably be something like the following?

  handleInputFocus: function(event) {
    this.props.onFocus(event);
    this.maybeShowList();
  },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants