Bugfix/UIPSelectSetting: empty option processing #152
Conversation
Sisha0
requested review from
ala-n,
nattallius,
yadamskaya,
AlexanderBazukevich and
PolinaVolchetskaya
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ |
src/utils/token-list-utils.ts
Outdated
| return Array.from(arrays.reduce((intersect, arr) => { | ||
| arr.forEach(val => !intersect.has(val) && intersect.delete(val)); | ||
| const sets = arrays.map(arr => new Set(arr)); | ||
| return Array.from(sets.reduce((intersect, set) => { |
There was a problem hiding this comment.
You can try the following snippet, if I remember correctly it has an optimal combination of speed and length
function intersection(a, b, ...rest) {
if (rest.length) return intersection(a, intersection(b, ...rest));
return a.filter(Set.prototype.has, new Set(b));
}
| @@ -114,8 +114,12 @@ export class UIPSelectSetting extends UIPSetting { | |||
|
|
|||
| /** Update setting's value for append {@link mode}. */ | |||
There was a problem hiding this comment.
/** Update setting's value for {@link mode} = "append" */
| @@ -114,8 +114,12 @@ export class UIPSelectSetting extends UIPSetting { | |||
|
|
|||
| /** Update setting's value for append {@link mode}. */ | |||
| protected updateAppend(attrValues: (string | null)[]): void { | |||
| const commonOptions = TokenListUtils.intersection( | |||
| ...attrValues.map(val => TokenListUtils.split(val)), this.settingOptions); | |||
| const optionsIntersections = attrValues.map(val => TokenListUtils.intersection(this.settingOptions, TokenListUtils.split(val))); | |||
There was a problem hiding this comment.
It's better to store this.settingOptions in the variable instead of creating it on each iteration
| @@ -114,8 +114,12 @@ export class UIPSelectSetting extends UIPSetting { | |||
|
|
|||
| /** Update setting's value for append {@link mode}. */ | |||
| protected updateAppend(attrValues: (string | null)[]): void { | |||
There was a problem hiding this comment.
Basically, the main problem with this method is that it's hard to understand what is going on.
Could you please try to add more comments, update variable names, and split the complex actions.
|
🎉 This PR is included in version 1.0.0-beta.9 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fix processing UIPSelectSetting's value when "empty" option (
<option value="">None</option>) is present