Add autocomplete module to 'confirm votes modal' - Closes #587 #625
Conversation
There was a problem hiding this comment.
It works like charm. A big piece of a good job.
Please, also, re-enable the e2e test of this feature
https://github.com/LiskHQ/lisk-nano/blob/2c3d897a6dbc27dc5c7ff463bc887c811395581c/features/voting.feature#L60-L67
This step will need to be updated
https://github.com/LiskHQ/lisk-nano/blob/2c3d897a6dbc27dc5c7ff463bc887c811395581c/features/step_definitions/voting.step.js#L15-L20
Unit test coverage of voteAutocomplete.js can be increased - look at the report - some else branches and mapDispatchToProps not covered.
Here's a refactoring suggestion if it's not too much work: voteAutocomplete.js has over 200 lines. A way to split it into two components would be to separate out a generic 'autocomplete' component which would be used twice in voteAutocomplete.js
src/utils/api/delegate.js
Outdated
| @@ -26,7 +36,9 @@ export const voteAutocomplete = (activePeer, username, votedDict) => { | |||
| return new Promise((resolve, reject) => | |||
| listDelegates(activePeer, options) | |||
| .then((response) => { | |||
| resolve(response.delegates.filter(d => !votedDict[d.username])); | |||
| resolve(response.delegates.filter(delegate => | |||
| !findDelegateInList(delegate.username, votedDict), | |||
There was a problem hiding this comment.
findDelegateInList is unnecessary. This can be just !votedDict.find(d => delegate.username === d.username), which can even be faster, as it doesn't go through the rest of the list if a delegate is found.
Also the argument votedDict was supposed to be a key-value dictionary (object), but since you use it for a list, votedList would be a more appropriate name.
| case 38: | ||
| this.handleArrowUp(this.state[listName], listName); | ||
| return false; | ||
| case 27 : |
There was a problem hiding this comment.
Please add a comment saying what key is 27
| [className]: styles.hidden, | ||
| }); | ||
| return false; | ||
| case 13 : |
There was a problem hiding this comment.
Please add a comment saying what key is 13
|
|
|
mapDispatchToProps is part of |
|
We don't tests |
Create a autocomplete module based on react-toolbox components and use it in confirm votes modal to complete migration of this component.