Feature: focused token listener

[lmgarret]

What issue does this pull request resolve?
It allows to react on changes of focus on tokens. Useful to display some information about the active option, as shown in the example.

What changes did you make?
Added a onTokenFocus prop to typeaheadContainer which takes an option and a focused boolean arg. The latter arg indicates a gain or loss of focus on the token representing the passed option.

Added a onTokenFocus to tokenContainer which takes only a focused boolean arg, indicating whether the token gained or lost focus.

Modified tokenContainer's _handleBlur and _handleActive methods to trigger this listener.

Added an example to the Behaviors section.

Is there anything that requires more attention while reviewing?
No.

[ericgio]

Hi @lmgarret, thanks for submitting this PR. Your solution is a little different than what was discussed in the original issue and I have a couple concerns:

1. This solution feels like it is addressing a relatively narrow use case (ie: yours) but doesn't necessarily provide a lot of additional flexibility. I'd prefer to keep things more low-level by making the onBlur, onClick, and onFocus handlers accept callbacks.
2. onTokenFocus doesn't seem important enough to add as a top-level prop on the component. Doing so adds complexity to the code since the prop needs to be passed down several levels. I'd prefer for the prop to only be exposed on the Token component, meaning you'd need to use renderToken to access it.

Any additional thoughts on this?

[lmgarret]

Hi!

1. Well I'd argue that it actually is a broader use case (which is still quite narrow in the end, as the initial suggestion). I first suggested to have a callback only when clicking on a Token, whereas here it covers the more general case of the Token being focused, including when clicked. Only listening to clicks created issues when toggling things like the message under the Typeahead in the example, and restricted the use case.

2. It was more convenient in my use case to have it as a top level props, but I can only agree with your point here.

Should I updated the PR according to (2.) or does (1.) makes you want to close it anyway?

[ericgio]

I first suggested to have a callback only when clicking on a Token

That's true. When thinking about your issue and use case, I was imagining adding onFocus and onBlur handlers in addition to the onClick handler you were requesting, but I never actually mentioned that in the issue. I apologize for not communicating that more clearly. That said, I think exposing the individual handlers will provide more flexibility and account for a broader range of use cases, despite being a bit less convenient for your particular use case.

Should I updated the PR according to (2.) or does (1.) makes you want to close it anyway?

If you're willing to try the approach I suggested, then by all means please go ahead and modify this PR or open a new one. However, I understand if that's not the direction you want to go with and prefer to abandon things.

If you do want to go ahead with the proposed changes, you can omit the example code and instead document the handlers in the Token API.

[ericgio]

onClick, onFocus, and onBlur handlers added to Token in b909ecc and released in v4.0.0-rc.1