Rework choice service and dialog

- cleans up the choice service and the choice dialogs to make them simpler and more usable. - adjusts search-value-selector to also accept lists - the search value selector allows for better filtering of models in the choice dialog - fixes an issue where deleting all tags required a selection
2019-10-23 10:41:58 +02:00 · 2019-10-23 10:41:58 +02:00 · 62eba77925
parent 62e5774c8d
commit 62eba77925
7 changed files with 149 additions and 193 deletions
--- a/client/src/app/core/ui-services/choice.service.ts
+++ b/client/src/app/core/ui-services/choice.service.ts
@ -1,12 +1,11 @@
 import { Injectable } from '@angular/core';
 import { MatDialog } from '@angular/material/dialog';

-import { mediumDialogSettings } from 'app/shared/utils/dialog-settings';
-import {
-    ChoiceAnswer,
-    ChoiceDialogComponent,
-    ChoiceDialogOptions
-} from '../../shared/components/choice-dialog/choice-dialog.component';
+import { Observable } from 'rxjs';
+
+import { infoDialogSettings } from 'app/shared/utils/dialog-settings';
+import { Displayable } from 'app/site/base/displayable';
+import { ChoiceAnswer, ChoiceDialogComponent } from '../../shared/components/choice-dialog/choice-dialog.component';

 /**
 * A service for prompting the user to select a choice.
@ -37,13 +36,13 @@ export class ChoiceService {
     */
    public async open(
        title: string,
-        choices: ChoiceDialogOptions,
+        choices?: Observable<Displayable[]> | Displayable[],
        multiSelect: boolean = false,
        actions?: string[],
        clearChoice?: string
    ): Promise<ChoiceAnswer> {
        const dialogRef = this.dialog.open(ChoiceDialogComponent, {
-            ...mediumDialogSettings,
+            ...infoDialogSettings,
            data: {
                title: title,
                choices: choices,
--- a/client/src/app/shared/components/choice-dialog/choice-dialog.component.html
+++ b/client/src/app/shared/components/choice-dialog/choice-dialog.component.html
@ -1,56 +1,52 @@
 <!-- Title -->
 <h2 mat-dialog-title>{{ data.title | translate }}</h2>

-<!-- Content -->
-<mat-dialog-content>
-    <mat-radio-group
-        #radio
-        name="choice"
-        *ngIf="!data.multiSelect && data.choices"
-        class="choice-radio-group"
-        [(ngModel)]="selectedChoice"
-    >
-        <mat-radio-button class="choice-button" *ngFor="let choice of data.choices" [value]="choice.id">
-            {{ getChoiceTitle(choice) | translate }}
-        </mat-radio-button>
-
-        <mat-divider *ngIf="data.clearChoice"></mat-divider>
-
-        <mat-radio-button *ngIf="data.clearChoice" [value]="null">
-            {{ data.clearChoice | translate }}
-        </mat-radio-button>
-    </mat-radio-group>
-
-    <mat-list *ngIf="data.multiSelect && data.choices">
-        <mat-list-item *ngFor="let choice of data.choices">
-            <mat-checkbox [checked]="isChosen(choice)" (change)="toggleChoice(choice)">
-                {{ getChoiceTitle(choice) | translate }}
-            </mat-checkbox>
-        </mat-list-item>
-    </mat-list>
-</mat-dialog-content>
-
-<!-- Actions -->
-<mat-dialog-actions>
-    <div *ngIf="data.actionButtons">
-        <button
-            *ngFor="let button of data.actionButtons"
-            mat-button
-            (click)="closeDialog(true, button)"
-            [disabled]="isSelectionEmpty"
+<form [formGroup]="selectForm">
+    <!-- Content -->
+    <div mat-dialog-content *ngIf="data.choices">
+        <os-search-value-selector
+            ngDefaultControl
+            [multiple]="data.multiSelect"
+            [formControl]="selectForm.get('select')"
+            [inputListValues]="data.choices"
        >
-            <span>{{ button | translate }}</span>
+        </os-search-value-selector>
+    </div>
+
+    <!-- Actions -->
+    <div mat-dialog-actions>
+        <!-- Custom Actions -->
+        <div *ngIf="data.actionButtons">
+            <button
+                *ngFor="let button of data.actionButtons"
+                mat-button
+                (click)="closeDialog(true, button)"
+                [disabled]="!hasSelection"
+            >
+                <span>{{ button | translate }}</span>
+            </button>
+        </div>
+
+        <!-- Normal okay button -->
+        <div *ngIf="!data.actionButtons">
+            <button
+                *ngIf="!data.multiSelect || data.choices"
+                mat-button
+                (click)="closeDialog(true)"
+                [disabled]="!hasSelection"
+            >
+                <span>OK</span>
+            </button>
+        </div>
+
+        <!-- Clear choice button -->
+        <button mat-button *ngIf="data.clearChoice" (click)="closeDialog(true, data.clearChoice)">
+            <span>{{ data.clearChoice }}</span>
+        </button>
+
+        <!-- Cancel -->
+        <button mat-button (click)="closeDialog(false)">
+            <span translate>Cancel</span>
        </button>
    </div>
-    <div *ngIf="!data.actionButtons">
-        <button
-            *ngIf="!data.multiSelect || data.choices.length"
-            mat-button
-            (click)="closeDialog(true)"
-            [disabled]="isSelectionEmpty"
-        >
-            <span>OK</span>
-        </button>
-    </div>
-    <button mat-button (click)="closeDialog(false)"><span translate>Cancel</span></button>
-</mat-dialog-actions>
+</form>
--- a/client/src/app/shared/components/choice-dialog/choice-dialog.component.scss
+++ b/client/src/app/shared/components/choice-dialog/choice-dialog.component.scss
@ -1,17 +1,3 @@
-.mat-dialog-content {
-    display: block;
-}
-
-.mat-radio-group {
-    display: inline-flex;
-    flex-direction: column;
-
-    .mat-radio-button {
-        margin: 5px;
-    }
-}
-
-.mat-divider {
-    margin-top: 10px;
-    margin-bottom: 10px;
+.mat-form-field {
+    width: 100%;
 }
--- a/client/src/app/shared/components/choice-dialog/choice-dialog.component.ts
+++ b/client/src/app/shared/components/choice-dialog/choice-dialog.component.ts
@ -1,20 +1,11 @@
 import { Component, Inject, ViewEncapsulation } from '@angular/core';
+import { FormBuilder, FormGroup } from '@angular/forms';
 import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog';

-import { Identifiable } from 'app/shared/models/base/identifiable';
+import { Observable } from 'rxjs';
+
 import { Displayable } from 'app/site/base/displayable';

-/**
- * An option needs to be identifiable and should have a strnig to display. Either uses Displayble or
- * a label property.
- */
-type ChoiceDialogOption = (Identifiable & Displayable) | (Identifiable & { label: string });
-
-/**
- * All choices in the array should have the same type.
- */
-export type ChoiceDialogOptions = (Identifiable & Displayable)[] | (Identifiable & { label: string })[];
-
 /**
 * All data needed for this dialog
 */
@ -24,16 +15,16 @@ interface ChoiceDialogData {
     */
    title: string;

-    /**
-     * The choices to display
-     */
-    choices: ChoiceDialogOptions;
-
    /**
     * Select if this should be a multiselect choice
     */
    multiSelect: boolean;

+    /**
+     * The choices to display
+     */
+    choices?: Observable<Displayable[]> | Displayable[];
+
    /**
     * Additional action buttons which will add their value to the
     * {@link closeDialog} feedback if chosen
@ -72,18 +63,24 @@ export class ChoiceDialogComponent {
    public selectedChoice: number;

    /**
-     * Checks if there is nothing selected
-     *
-     * @returns true if there is no selection chosen (and the dialog should not
-     * be closed 'successfully')
+     * Form to hold the selection
     */
-    public get isSelectionEmpty(): boolean {
-        if (this.data.multiSelect) {
-            return this.selectedMultiChoices.length === 0;
-        } else if (!this.data.choices) {
-            return false;
+    public selectForm: FormGroup;
+
+    /**
+     * Checks if there is something selected
+     *
+     * @returns true if there is a selection chosen
+     */
+    public get hasSelection(): boolean {
+        if (this.data.choices) {
+            if (this.selectForm.get('select').value) {
+                return !!this.selectForm.get('select').value || !!this.selectForm.get('select').value.length;
+            } else {
+                return false;
+            }
        } else {
-            return this.selectedChoice === undefined;
+            return true;
        }
    }

@ -94,21 +91,12 @@ export class ChoiceDialogComponent {

    public constructor(
        public dialogRef: MatDialogRef<ChoiceDialogComponent>,
+        private formBuilder: FormBuilder,
        @Inject(MAT_DIALOG_DATA) public data: ChoiceDialogData
-    ) {}
-
-    /**
-     * Get the title from a choice. Maybe saved in a label property or using getTitle().
-     *
-     * @param choice The choice
-     * @return the title
-     */
-    public getChoiceTitle(choice: ChoiceDialogOption): string {
-        if ('label' in choice) {
-            return choice.label;
-        } else {
-            return choice.getTitle();
-        }
+    ) {
+        this.selectForm = this.formBuilder.group({
+            select: []
+        });
    }

    /**
@ -121,31 +109,10 @@ export class ChoiceDialogComponent {
        if (ok) {
            this.dialogRef.close({
                action: action ? action : null,
-                items: this.data.multiSelect ? this.selectedMultiChoices : this.selectedChoice
+                items: this.selectForm.get('select').value
            });
        } else {
            this.dialogRef.close();
        }
    }
-
-    /**
-     * For multiSelect: Determines whether a choice has been activated
-     * @param choice
-     */
-    public isChosen(choice: Identifiable): boolean {
-        return this.selectedMultiChoices.indexOf(choice.id) >= 0;
-    }
-
-    /**
-     * For multiSelect: Activates/deactivates a multi-Choice option
-     * @param choice
-     */
-    public toggleChoice(choice: Identifiable): void {
-        const idx = this.selectedMultiChoices.indexOf(choice.id);
-        if (idx < 0) {
-            this.selectedMultiChoices.push(choice.id);
-        } else {
-            this.selectedMultiChoices.splice(idx, 1);
-        }
-    }
 }
--- a/client/src/app/shared/components/search-value-selector/search-value-selector.component.ts
+++ b/client/src/app/shared/components/search-value-selector/search-value-selector.component.ts
@ -79,23 +79,27 @@ export class SearchValueSelectorComponent implements OnDestroy {
     * changes its values.
     */
    @Input()
-    public set inputListValues(value: Observable<Selectable[]>) {
+    public set inputListValues(value: Selectable[] | Observable<Selectable[]>) {
        if (!value) {
            return;
        }
-        // unsubscribe to old subscription.
-        if (this._inputListSubscription) {
-            this._inputListSubscription.unsubscribe();
-        }
-        // this.inputSubject = value;
-        this._inputListSubscription = value.pipe(auditTime(10)).subscribe(items => {
-            this.selectableItems = items;
-            if (this.formControl) {
-                !!items && items.length > 0
-                    ? this.formControl.enable({ emitEvent: false })
-                    : this.formControl.disable({ emitEvent: false });
+
+        if (Array.isArray(value)) {
+            this.selectableItems = value;
+        } else {
+            // unsubscribe to old subscription.
+            if (this._inputListSubscription) {
+                this._inputListSubscription.unsubscribe();
            }
-        });
+            this._inputListSubscription = value.pipe(auditTime(10)).subscribe(items => {
+                this.selectableItems = items;
+                if (this.formControl) {
+                    !!items && items.length > 0
+                        ? this.formControl.enable({ emitEvent: false })
+                        : this.formControl.disable({ emitEvent: false });
+                }
+            });
+        }
    }

    /**
--- a/client/src/app/site/motions/modules/category/components/category-motions-sort/category-motions-sort.component.ts
+++ b/client/src/app/site/motions/modules/category/components/category-motions-sort/category-motions-sort.component.ts
@ -214,25 +214,23 @@ export class CategoryMotionsSortComponent extends BaseViewComponent implements O

    public async moveToPosition(): Promise<void> {
        if (this.sortSelector.multiSelectedIndex.length) {
-        }
-        const content = this.translate.instant('Move selected items ...');
-        const choices = this.sortSelector.sortedItems
-            .map((item, index) => {
-                return { id: index, label: item.getTitle() };
-            })
-            .filter(f => !this.sortSelector.multiSelectedIndex.includes(f.id));
-        const actions = [this.translate.instant('Insert before'), this.translate.instant('Insert behind')];
-        const selectedChoice = await this.choiceService.open(content, choices, false, actions);
-        if (selectedChoice) {
-            const newIndex = selectedChoice.items as number;
-
-            this.sortSelector.drop(
-                {
-                    currentIndex: newIndex,
-                    previousIndex: null
-                },
-                selectedChoice.action === actions[1] // true if 'insert behind'
+            const content = this.translate.instant('Move selected items ...');
+            const choices = this.sortSelector.sortedItems.filter(
+                f => !this.sortSelector.multiSelectedIndex.includes(f.id)
            );
+            const actions = [this.translate.instant('Insert before'), this.translate.instant('Insert behind')];
+            const selectedChoice = await this.choiceService.open(content, choices, false, actions);
+            if (selectedChoice) {
+                const newIndex = selectedChoice.items as number;
+
+                this.sortSelector.drop(
+                    {
+                        currentIndex: newIndex,
+                        previousIndex: null
+                    },
+                    selectedChoice.action === actions[1] // true if 'insert behind'
+                );
+            }
        }
    }
 }
--- a/client/src/app/site/motions/services/motion-multiselect.service.ts
+++ b/client/src/app/site/motions/services/motion-multiselect.service.ts
@ -15,8 +15,6 @@ import { OverlayService } from 'app/core/ui-services/overlay.service';
 import { PersonalNoteService } from 'app/core/ui-services/personal-note.service';
 import { PromptService } from 'app/core/ui-services/prompt.service';
 import { TreeService } from 'app/core/ui-services/tree.service';
-import { ChoiceDialogOptions } from 'app/shared/components/choice-dialog/choice-dialog.component';
-import { Identifiable } from 'app/shared/models/base/identifiable';
 import { Displayable } from 'app/site/base/displayable';
 import { ViewMotion } from '../models/view-motion';

@ -93,7 +91,7 @@ export class MotionMultiselectService {
     */
    public async moveToItem(motions: ViewMotion[]): Promise<void> {
        const title = this.translate.instant('This will move all selected motions as childs to:');
-        const choices: (Displayable & Identifiable)[] = this.agendaRepo.getViewModelList();
+        const choices = this.agendaRepo.getViewModelListObservable();
        const selectedChoice = await this.choiceService.open(title, choices);
        if (selectedChoice) {
            const requestData = {
@ -112,10 +110,7 @@ export class MotionMultiselectService {
    public async setStateOfMultiple(motions: ViewMotion[]): Promise<void> {
        if (motions.every(motion => motion.workflow_id === motions[0].workflow_id)) {
            const title = this.translate.instant('This will set the following state for all selected motions:');
-            const choices = this.workflowRepo.getWorkflowStatesForMotions(motions).map(workflowState => ({
-                id: workflowState.id,
-                label: workflowState.name
-            }));
+            const choices = this.workflowRepo.getWorkflowStatesForMotions(motions);
            const selectedChoice = await this.choiceService.open(title, choices);
            if (selectedChoice) {
                const message = `${motions.length} ` + this.translate.instant(this.messageForSpinner);
@ -137,12 +132,16 @@ export class MotionMultiselectService {
            const title = this.translate.instant(
                'This will set the following recommendation for all selected motions:'
            );
-            const choices = this.workflowRepo
+
+            // hacks custom Displayables from recommendations
+            // TODO: Recommendations should be an own class
+            const choices: Displayable[] = this.workflowRepo
                .getWorkflowStatesForMotions(motions)
                .filter(workflowState => !!workflowState.recommendation_label)
                .map(workflowState => ({
                    id: workflowState.id,
-                    label: workflowState.recommendation_label
+                    getTitle: () => workflowState.recommendation_label,
+                    getListTitle: () => workflowState.recommendation_label
                }));
            const clearChoice = this.translate.instant('Delete recommendation');
            const selectedChoice = await this.choiceService.open(title, choices, false, null, clearChoice);
@ -174,7 +173,7 @@ export class MotionMultiselectService {
        const clearChoice = this.translate.instant('No category');
        const selectedChoice = await this.choiceService.open(
            title,
-            this.categoryRepo.getViewModelList(),
+            this.categoryRepo.getViewModelListObservable(),
            false,
            null,
            clearChoice
@ -196,7 +195,12 @@ export class MotionMultiselectService {
            'This will add or remove the following submitters for all selected motions:'
        );
        const choices = [this.translate.instant('Add'), this.translate.instant('Remove')];
-        const selectedChoice = await this.choiceService.open(title, this.userRepo.getViewModelList(), true, choices);
+        const selectedChoice = await this.choiceService.open(
+            title,
+            this.userRepo.getViewModelListObservable(),
+            true,
+            choices
+        );
        if (selectedChoice) {
            let requestData = null;
            if (selectedChoice.action === choices[0]) {
@ -232,12 +236,14 @@ export class MotionMultiselectService {
     */
    public async changeTags(motions: ViewMotion[]): Promise<void> {
        const title = this.translate.instant('This will add or remove the following tags for all selected motions:');
-        const choices = [
-            this.translate.instant('Add'),
-            this.translate.instant('Remove'),
+        const choices = [this.translate.instant('Add'), this.translate.instant('Remove')];
+        const selectedChoice = await this.choiceService.open(
+            title,
+            this.tagRepo.getViewModelListObservable(),
+            true,
+            choices,
            this.translate.instant('Clear tags')
-        ];
-        const selectedChoice = await this.choiceService.open(title, this.tagRepo.getViewModelList(), true, choices);
+        );
        if (selectedChoice) {
            let requestData = null;
            if (selectedChoice.action === choices[0]) {
@ -258,7 +264,7 @@ export class MotionMultiselectService {
                        tags: tagIds
                    };
                });
-            } else if (selectedChoice.action === choices[2]) {
+            } else {
                requestData = motions.map(motion => {
                    return {
                        id: motion.id,
@ -283,7 +289,7 @@ export class MotionMultiselectService {
        const clearChoice = this.translate.instant('Clear motion block');
        const selectedChoice = await this.choiceService.open(
            title,
-            this.motionBlockRepo.getViewModelList(),
+            this.motionBlockRepo.getViewModelListObservable(),
            false,
            null,
            clearChoice
@ -347,16 +353,16 @@ export class MotionMultiselectService {
     */
    public async bulkSetFavorite(motions: ViewMotion[]): Promise<void> {
        const title = this.translate.instant('This will set the favorite status for all selected motions:');
-        const choices: ChoiceDialogOptions = [
-            { id: 1, label: this.translate.instant('Set as favorite') },
-            { id: 2, label: this.translate.instant('Set as not favorite') }
-        ];
-        const selectedChoice = await this.choiceService.open(title, choices);
+        const options = [this.translate.instant('Set as favorite'), this.translate.instant('Set as not favorite')];
+        const selectedChoice = await this.choiceService.open(title, null, false, options);
        if (selectedChoice && motions.length) {
+            /**
+             * `bulkSetStar` does imply that "true" sets favorites while "false" unsets favorites
+             */
+            const setOrUnset = selectedChoice.action === options[0];
            const message = this.translate.instant(`I have ${motions.length} favorite motions. Please wait ...`);
-            const star = (selectedChoice.items as number) === choices[0].id;
            this.overlayService.showSpinner(message, true);
-            await this.personalNoteService.bulkSetStar(motions, star);
+            await this.personalNoteService.bulkSetStar(motions, setOrUnset);
        }
    }
 }