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. |
![]() |
|
Col: 22 Expected ')' and instead saw ';'. |
![]() |
|
Col: 21 Expected an identifier and instead saw ')'. |
![]() |
|
Col: 21 Unexpected ')'. |
![]() |
|
Col: 22 Expected ')' and instead saw ';'. |
![]() |
|
Col: 23 Missing semicolon. |
![]() |
|
Col: 21 Unexpected ')'. |
![]() |
|
Col: 21 Expected an identifier and instead saw ')'. |
![]() |
|
I think with RB 3.0+, we can assume localeCompare exists. |
|
|
Why did you pull this out? |
|
|
Trailing whitespace. |
|
|
Add a trailing comma? |
|
|
Add a trailing comma? |
|
|
this.options.initialOptions.forEach(item => this._onItemSelected(item, false)); |
|
|
No spaces around the = |
|
|
Add a trailing comma? |
|
|
Add a trailing comma? |
|
|
Add a trailing comma? |
|
|
Add a trailing comma? |
|
|
Add a trailing comma? |
|
|
Add a trailing comma? |
|
|
Col: 3 Missing semicolon. |
![]() |
|
While it was like this before, the placeholder text needs to come from the subclass, since the generic implementation shouldn't … |
|
|
Why did you create a default value for this? Is it valid to construct this without passing in options? At … |
|
|
No spaces around =. Same question here about whether options should really be optional. |
|
|
Would be a little bit better as: .click(() => this._onItemRemoved($li, item)); |
|
- 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.