We create callback functions in tooltip component and save them to state in context.
I also created state for keeping information about position of currently chosen button.
User interactions (pressing buttons and hovering mouse) will use callbacks to change internal state of component.
Details
Details
Typeahead without keyboard support works.
Final tests performed in last diffs.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Spotted bug later on, that allowed user to accept @-ing with enter when overlay was no longer visible. Clearing callbacks on unmount fixes it.
Comment Actions
Spotted a bug: We were not setting chosen position to 0, when suggested users changed.
| web/chat/typeahead-tooltip.react.js | ||
|---|---|---|
| 39–41 | Are we sure we would like to reset the selection when this array changes? Resetting like that would also be executed when any user changes - e.g. its name or role is updated. | |
| 78–80 | Shouldn't we keep this? | |
| 100 | Just a nit, but maybe it will be better to rename this to e.g. execute? It makes more sense to execute an action than to "onclick" it. | |
| 105 | What happens when actions array becomes empty? | |
| 119–122 | ||
| 125–132 | ||
| web/chat/typeahead-tooltip.react.js | ||
|---|---|---|
| 39–41 | In diff 13/13 I solve this problem by freezing thread users that are later matched against typed prefix. So the list of users won't change when tooltip is visible. We set position to 0, when suggestion change to handle situation, when users types "@a" moves down the list and the types another letter (that causes change of suggested users). | |
| 78–80 | As mentioned above, actions can never be empty or null, so it is unnecessary. | |
| 100 | I like it | |
| 105 | It will never be empty. Number of actions is equal to number of suggested users and we only render overlay if that number is greater than 0. Maybe I should add invariant inside of component so we are sure we work with that condition fulfilled? | |
| web/chat/typeahead-tooltip.react.js | ||
|---|---|---|
| 39–41 | Not really. Say we type another letter, but list of suggested users doesn't change. In this situation we don't really need to move to the top of the list. We could, but I found it unnecessary. | |
| 105 | Added ifs in thos callbacks and if after all memoizations to check if component received nonempty suggestedusers array as a final check. | |