chore: code review, handle sort correctly

alauda · Mar 1, 2022 · 96545d6 · 96545d6
1 parent dd04992
commit 96545d6
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 53 deletions.
diff --git a/src/select/multi-select/multi-select.component.html b/src/select/multi-select/multi-select.component.html
@@ -37,7 +37,7 @@
     [round]="true"
     [border]="true"
     [size]="tagSize"
-    [closeable]="isOptionRemovable(option)"
+    [closeable]="!disabled && !option.disabled"
     (close)="removeValue(option.value)"
     [ngClass]="tagClassFn && tagClassFn(option.label, option.value)"
   >

diff --git a/src/select/multi-select/multi-select.component.ts b/src/select/multi-select/multi-select.component.ts
@@ -166,53 +166,58 @@ export class MultiSelectComponent<T = unknown>
         switchMap((options: QueryList<OptionComponent<T>>) =>
           options.length > 0
             ? combineLatest(
-                [...options]
-                  .sort((a, b) => {
-                    if (a.selected && a.disabled) {
-                      return -1;
-                    }
-
-                    if (b.selected && b.disabled) {
-                      return 1;
-                    }
-
-                    return 0;
-                  })
-                  .map(option =>
-                    combineLatest([
-                      option.value$,
-                      option.label$,
-                      option.labelContext$,
-                    ]).pipe(
-                      map(([value, label, labelContext]) => ({
-                        value,
-                        label,
-                        labelContext,
-                      })),
-                    ),
+                options.map(option =>
+                  combineLatest([
+                    option.value$,
+                    option.label$,
+                    option.labelContext$,
+                    option.disabled$,
+                  ]).pipe(
+                    map(([value, label, labelContext, disabled]) => ({
+                      value,
+                      label,
+                      labelContext,
+                      disabled,
+                    })),
                   ),
+                ),
               )
             : of([] as Array<SelectFilterOption<T>>),
         ),
       ),
     ]).pipe(
       map(([values, options]) =>
-        values.map(value => {
-          const option = options.find(
-            option => this.trackFn(option.value) === this.trackFn(value),
-          );
-          return option
-            ? {
-                label: option.label || coerceString(this.trackFn(option.value)),
-                labelContext: option.labelContext,
-                value: option.value,
-              }
-            : {
-                label:
-                  this.labelFn?.(value) || coerceString(this.trackFn(value)),
-                value,
-              };
-        }),
+        values
+          .map(value => {
+            const option = options.find(
+              option => this.trackFn(option.value) === this.trackFn(value),
+            );
+            return option
+              ? {
+                  label:
+                    option.label || coerceString(this.trackFn(option.value)),
+                  labelContext: option.labelContext,
+                  value: option.value,
+                  disabled: option.disabled,
+                }
+              : {
+                  label:
+                    this.labelFn?.(value) || coerceString(this.trackFn(value)),
+                  value,
+                };
+          })
+          // sort disabled options as first
+          .sort((a, b) => {
+            if (a.disabled) {
+              return -1;
+            }
+
+            if (b.disabled) {
+              return 1;
+            }
+
+            return 0;
+          }),
       ),
       publishReplay(1),
       refCount(),
@@ -316,13 +321,6 @@ export class MultiSelectComponent<T = unknown>
     });
   }
 
-  isOptionRemovable(option: SelectFilterOption<T>) {
-    const currOption = this.contentOptions.find(
-      opt => this.trackFn(opt.value) === this.trackFn(option.value),
-    );
-    return !this.disabled && !currOption.disabled;
-  }
-
   selectOption(option: OptionComponent<T>) {
     if (option.selected) {
       this.removeValue(option.value);

diff --git a/src/select/option/option.component.ts b/src/select/option/option.component.ts
@@ -44,6 +44,8 @@ export class OptionComponent<T> {
 
   private readonly value$$ = new BehaviorSubject(this.value);
 
+  private readonly disabled$$ = new BehaviorSubject(this.disabled);
+
   @Input()
   get label() {
     return this._label;
@@ -75,12 +77,13 @@ export class OptionComponent<T> {
   }
 
   @Input()
-  get disabled() {
+  get disabled(): boolean {
     return this._disabled;
   }
 
   set disabled(val: boolean | '') {
     this._disabled = coerceAttrBoolean(val);
+    this.disabled$$.next(this._disabled);
   }
 
   isMulti = false;
@@ -97,6 +100,7 @@ export class OptionComponent<T> {
   value$ = this.value$$.asObservable();
   label$ = this.label$$.asObservable();
   labelContext$ = this.labelContext$$.asObservable();
+  disabled$ = this.disabled$$.asObservable();
 
   selected$: Observable<boolean>;
   size$: Observable<ComponentSize>;

diff --git a/src/select/select.component.ts b/src/select/select.component.ts
@@ -68,13 +68,15 @@ export class SelectComponent<T = unknown>
   isClearable = (hasSelected: boolean) =>
     !this.disabled && this.clearable && hasSelected;
 
-    override ngAfterContentInit() {
+  override ngAfterContentInit() {
     super.ngAfterContentInit();
 
     this.selectedOption$ = combineLatest([
-      this.contentOptions.changes.pipe(
+      (
+        this.contentOptions.changes as Observable<QueryList<OptionComponent<T>>>
+      ).pipe(
         startWith(this.contentOptions),
-        switchMap((options: QueryList<OptionComponent<T>>) =>
+        switchMap(options =>
           combineLatest(options.map(option => option.selected$)).pipe(
             startWith(null as void),
             map(() => options.find(option => option.selected)),

diff --git a/src/select/select.types.ts b/src/select/select.types.ts
@@ -6,6 +6,7 @@ export interface SelectOption {
 }
 
 export interface SelectFilterOption<T> extends SelectOption {
+  disabled?: boolean;
   value: T;
 }
 
@@ -18,7 +19,7 @@ export type TrackFn<T, R = unknown> = (value: T) => R;
 
 export type TagClassFn<
   V,
-  T extends string | TemplateRef<unknown> = string | TemplateRef<unknown>
+  T extends string | TemplateRef<unknown> = string | TemplateRef<unknown>,
 > = (
   label: T,
   value: V,