Suggestions: add "use current input" command #3929
Conversation
|
@joschect I'd love some input on this one! |
| } | ||
|
|
||
| @autobind | ||
| public tryHandleKeyDown(keyCode: number, currentSuggestionIndex: number): boolean { |
There was a problem hiding this comment.
Add a comment that return true means it was handled, and false means it wasn't
There was a problem hiding this comment.
This entire function seems like it should be in the controller.
There was a problem hiding this comment.
How do I move this into the controller? The suggestionsController doesn't seem to be aware of the other elements in the suggestion element? (I may be missing something)
There was a problem hiding this comment.
It's not really supposed to be aware. That said it makes sense that it would need to be aware in order to manage stuff like this. Perhaps the answer is to make SuggestionsController a react element whose render function only returns <Suggestions>. That way it can have a ref to suggestions, which in turn exposes an api for stuff like clicking the "search more" button. The main goal, in my mind, is to keep the "state manager" (SuggestionsController, maybe it should get renamed?) separate from the "view" (Suggestions). That way it's easy to mix and match the two and each one is clean, there isn't a conflating of state and form.
| @autobind | ||
| private _useInput() { | ||
| if (this.props.createGenericItem) { | ||
| this.props.createGenericItem(); |
There was a problem hiding this comment.
this feels more like it should be forceResolve or something like that.
There was a problem hiding this comment.
yes, that sounds like a much better name, I'll change that where applicable
2 participants
Pull request checklist
$ npm run changeDescription of changes