Port the related user selector to ES6
Review Request #8984 — Created June 2, 2017 and submitted
This change ports the related user & object selctor JS to use ES6.
The related user selection widget on the repositories change form
still works as intended.
Description | From | Last Updated |
---|---|---|
Col: 23 Missing semicolon. |
reviewbot | |
Col: 22 Expected ')' and instead saw ';'. |
reviewbot | |
Col: 21 Expected an identifier and instead saw ')'. |
reviewbot | |
Col: 21 Unexpected ')'. |
reviewbot | |
Col: 22 Expected ')' and instead saw ';'. |
reviewbot | |
Col: 23 Missing semicolon. |
reviewbot | |
Col: 21 Unexpected ')'. |
reviewbot | |
Col: 21 Expected an identifier and instead saw ')'. |
reviewbot | |
I think with RB 3.0+, we can assume localeCompare exists. |
david | |
Why did you pull this out? |
david | |
Trailing whitespace. |
david | |
Add a trailing comma? |
david | |
Add a trailing comma? |
david | |
this.options.initialOptions.forEach(item => this._onItemSelected(item, false)); |
david | |
No spaces around the = |
david | |
Add a trailing comma? |
david | |
Add a trailing comma? |
david | |
Add a trailing comma? |
david | |
Add a trailing comma? |
david | |
Add a trailing comma? |
david | |
Add a trailing comma? |
david | |
Col: 3 Missing semicolon. |
reviewbot | |
While it was like this before, the placeholder text needs to come from the subclass, since the generic implementation shouldn't … |
david | |
Why did you create a default value for this? Is it valid to construct this without passing in options? At … |
david | |
No spaces around =. Same question here about whether options should really be optional. |
david | |
Would be a little bit better as: .click(() => this._onItemRemoved($li, item)); |
david |
- Description:
-
~ This change ports the related user & object selctor JS to use ES6. This
~ This change ports the related user & object selctor JS to use ES6.
- is part of a multi-patch refactor to allow the user selector to support - both single-valued and multi-valued inputs. While porting the widget, I refactored
RB.RelatedObjectSelectorView.compareStrings
function, which previouslywould check for the presence of String.prototype.localeCompare
on eachcall, into a thin wrapper around either localeCompare
(if defined) orthe fallback method. This way we only have to check for localeCompare
's presence once.
-
-
While it was like this before, the placeholder text needs to come from the subclass, since the generic implementation shouldn't have "Search for users"
-
Why did you create a default value for this? Is it valid to construct this without passing in options?
At the least, it looks like we'd need to have a default value for
selectizeOptions
(since_.defaults(undefined, { ... })
will returnundefined
). -
- Description:
-
~ This change ports the related user & object selctor JS to use ES6.
~ This change ports the related user & object selctor JS to use ES6.
- - While porting the widget, I refactored
- RB.RelatedObjectSelectorView.compareStrings
function, which previously- would check for the presence of String.prototype.localeCompare
on each- call, into a thin wrapper around either localeCompare
(if defined) or- the fallback method. This way we only have to check for - localeCompare
's presence once.