Using callback functions to change chosen button, accept it or close overlay.
Details
Details
Tested how it all fits together. Results on a video (currently without scrolling overlay content - added in next diff):
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
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 |
Comment Actions
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. |
Comment Actions
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. |