Conversation
McDoyen
commented
Nov 26, 2018
- Dropdown type ahead
…pdown-type-ahead into tests/end-to-end
…pdown-type-ahead into tests/end-to-end
| } else { | ||
| this.getSelectedValue(this.props); | ||
|
|
||
| return Promise.resolve({ options: [] }); |
There was a problem hiding this comment.
This line is not necessary. The last return will be called
| } else { | ||
| this.getSelectedValues(this.props); | ||
|
|
||
| return Promise.resolve({ options: [] }); |
There was a problem hiding this comment.
This line is not necessary. The last return will be called
| const dropdownElement = dropdown[0] as HTMLElement; | ||
| if (dropdownElement && dropdownElement.style.visibility !== "visible") { | ||
| const dropdownDimensions = dropdown[0].getBoundingClientRect(); | ||
| if (dropdownElement && dropdownDimensions) { |
There was a problem hiding this comment.
dropdownElement is already checked at line 102, if you just use if (dropdownDimensions){ it will have the same effect
| const dropdown = document.getElementsByClassName("Select-menu-outer"); | ||
| const dropdownElement = dropdown[0] as HTMLElement; | ||
| if (dropdownElement && dropdownElement.style.visibility !== "visible") { | ||
| const dropdownDimensions = dropdown[0].getBoundingClientRect(); |
There was a problem hiding this comment.
dropdown[0] is equivalent to dropdownElement. Use const dropdownDimensions = dropdownElement.getBoundingClientRect();
| .then((newValue: any) => { | ||
| if (newValue) { | ||
| this.setState({ | ||
| isClearable: this.props.isClearable ? true : false, |
There was a problem hiding this comment.
You can use !!this.props.isClearable instead of this.props.isClearable ? true : false
| const dropdownElement = dropdown[0] as HTMLElement; | ||
| if (dropdownElement && dropdownElement.style.visibility !== "visible") { | ||
| const dropdownDimensions = dropdownElement.getBoundingClientRect(); | ||
| if (dropdownElement && dropdownDimensions) { |
There was a problem hiding this comment.
dropdownElement is already checked at line 117. Use if (dropdownDimensions) { instead
|
|
||
| if (this.props.selectedValue.length > 0) { | ||
| formatedOptions = this.props.selectedValue.map((selectedGuid: any) => { | ||
| if (this.props.selectedValue) { |
There was a problem hiding this comment.
This line will always be true, because it's inside the map of itself! It needs to be merged with line 141.
Suggestion: remove the line 143 and at line 141 use: if (this.props.selectedValue && this.props.selectedValue.length > 0) {
| if (this.props.selectedValue.length > 0) { | ||
| formatedOptions = this.props.selectedValue.map((selectedGuid: any) => { | ||
| if (this.props.selectedValue) { | ||
| this.props.selectedValue.forEach((dataObject: any) => { |
There was a problem hiding this comment.
@McDoyen maybe I'm wrong, but this block of code (lines 142 to 153), looks like a forEach in the same list of objects to verify if the value is equal and then return the label, but here we have 2 problems:
- the forEach is never stopped when it finds the equivalent to the value
- the number of interactions is this.props.selectedValue.length ^ 2
Instead of that, if is just to return an array of selected labels, use this:
formatedOptions = this.props.selectedValue.map((selectedGuid: any) => selectedGuid.label);
If my understanding is wrong, add a return; at line 148
14a6ff9 to
55b9f0b
Compare
55b9f0b to
4e28a45
Compare
6af20b8 to
6cea6bd
Compare
6cea6bd to
79448b0
Compare
cabca2e to
7b487f0
Compare
b0eccb5 to
a38e3e2
Compare
a38e3e2 to
a8299ed
Compare
35e5747 to
61f3673
Compare
6f325a7 to
650c76f
Compare
650c76f to
7349ba7
Compare