Using callback functions to change chosen button, accept it or close overlay.
Details
Tested how it all fits together. Results on a video (currently without scrolling overlay content - added in next diff):
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
web/chat/chat-input-bar.react.js | ||
---|---|---|
419–422 ↗ | (On Diff #20178) | I think the destructure syntax can be used here, but however you prefer. I find this to look cleaner |
Destructure as Inka suggested.
web/chat/chat-input-bar.react.js | ||
---|---|---|
419–422 ↗ | (On Diff #20178) | Right, I haven't thought about using it in this context, thanks for suggestion! |
web/chat/chat-input-bar.react.js | ||
---|---|---|
425–451 ↗ | (On Diff #20513) | You can reduce duplication in this code by creating a map from key to function and using it instead of else if chain. Something like the included code, but it should be possible to simplify it even further. |
web/utils/typeahead-utils.js | ||
127 ↗ | (On Diff #20513) | |
127–129 ↗ | (On Diff #20513) | Why only one prop has overlay in name? Do we need it? |
129 ↗ | (On Diff #20513) | Can we keep it next to setChosenPositionInOverlay? |
133–141 ↗ | (On Diff #20513) | It should work and doesn't need to be changed, but it would be less fragile if the logic was using keys instead of idx. |
Responded to inlines and fixed code.
Tested everything once again.
web/chat/chat-input-bar.react.js | ||
---|---|---|
425–451 ↗ | (On Diff #20513) | Thanks for the suggestion! I felt like it is really repetitive, but couldn't find a way to simplify it. |
web/utils/typeahead-utils.js | ||
127–129 ↗ | (On Diff #20513) | I forgot to rename chosenActionPosition argument. Renamed it so it match name from callsite and correspond to its setter. |
133–141 ↗ | (On Diff #20513) | I'm not sure if we can use keys here. They are based on user id: key: suggestedUser.id, as you can see in getTypeaheadTooltipActions function above. And chosenPositionInOverlay is simply index in list which doesn't really correspond to user id. |