Merge pull request #5264 from tsiegleauq/bar-chart-rework

Rework Chart component

client/src/app/shared/components/charts/charts.component.html

@@ -1,26 +1,15 @@
-<div class="charts-wrapper" [ngClass]="[classes, hasPadding ? 'has-padding' : '']">
+<div class="charts-wrapper" [ngClass]="[hasPadding ? 'has-padding' : '']">
-    <ng-container *ngIf="chartData.length || circleData.length">
+    <canvas
-        <canvas
+        class="chart-js-canvas"
-            *ngIf="type === 'bar' || type === 'stackedBar' || type === 'horizontalBar' || type === 'line'"
+        *ngIf="chartData && chartData.length"
-            baseChart
+        baseChart
-            [datasets]="chartData"
+        [datasets]="isCircle ? null : chartData"
-            [labels]="labels"
+        [data]="isCircle ? chartData : null"
-            [legend]="showLegend"
+        [colors]="circleColors"
-            [options]="chartOptions"
+        [labels]="labels"
-            [chartType]="type"
+        [options]="chartOptions"
-            (chartClick)="select.emit($event)"
+        [chartType]="type"
-            (chartHover)="hover.emit($event)"
+        [legend]="legend"
-        >
+    >
-        </canvas>
+    </canvas>
-        <canvas
-            *ngIf="type === 'pie' || type === 'doughnut'"
-            baseChart
-            [options]="pieChartOptions"
-            [data]="circleData"
-            [labels]="circleLabels"
-            [colors]="circleColors"
-            [chartType]="type"
-            [legend]="showLegend"
-        ></canvas>
-    </ng-container>
 </div>

client/src/app/shared/components/charts/charts.component.scss

@@ -7,9 +7,3 @@
         padding: 16px;
     }
 }
-
-@for $i from 1 through 100 {
-    .os-charts--#{$i} {
-        width: unquote($string: $i + '%');
-    }
-}

client/src/app/shared/components/charts/charts.component.ts

@@ -1,26 +1,17 @@
-import { ChangeDetectionStrategy, ChangeDetectorRef, Component, EventEmitter, Input, Output } from '@angular/core';
+import { ChangeDetectionStrategy, Component, Input } from '@angular/core';
 import { MatSnackBar } from '@angular/material';
 import { Title } from '@angular/platform-browser';
 
 import { TranslateService } from '@ngx-translate/core';
 import { ChartOptions } from 'chart.js';
 import { Label } from 'ng2-charts';
-import { Observable } from 'rxjs';
 
 import { BaseViewComponent } from 'app/site/base/base-view';
 
 /**
  * The different supported chart-types.
  */
-export type ChartType = 'line' | 'bar' | 'pie' | 'doughnut' | 'horizontalBar' | 'stackedBar';
+export type ChartType = 'doughnut' | 'pie' | 'horizontalBar';
-
-/**
- * Describes the events the chart is fired, when hovering or clicking on it.
- */
-interface ChartEvent {
-    event: MouseEvent;
-    active: {}[];
-}
 
 /**
  * One single collection in an array.
@@ -39,8 +30,6 @@ export interface ChartDate {
  */
 export type ChartData = ChartDate[];
 
-export type ChartLegendSize = 'small' | 'middle';
-
 /**
  * Wrapper for the chart-library.
  *
@@ -54,75 +43,17 @@ export type ChartLegendSize = 'small' | 'middle';
 })
 export class ChartsComponent extends BaseViewComponent {
     /**
-     * Sets the data as an observable.
+     * The type of the chart.
-     *
-     * The data is prepared and splitted to dynamic use of bar/line or doughnut/pie chart.
      */
     @Input()
-    public set data(dataObservable: Observable<ChartData>) {
+    public type: ChartType = 'horizontalBar';
-        this.subscriptions.push(
-            dataObservable.subscribe(data => {
-                if (!data) {
-                    return;
-                }
-                data = data.flatMap((date: ChartDate) => ({ ...date, data: date.data.filter(value => value >= 0) }));
-                this.chartData = data;
-                this.circleData = data.flatMap((date: ChartDate) => date.data);
-                this.circleLabels = data.map(date => date.label);
-                const circleColors = [
-                    {
-                        backgroundColor: data.map(date => date.backgroundColor).filter(color => !!color),
-                        hoverBackgroundColor: data.map(date => date.hoverBackgroundColor).filter(color => !!color)
-                    }
-                ];
-                this.circleColors = !!circleColors[0].backgroundColor.length ? circleColors : null;
-                this.checkAndUpdateChartType();
-                this.cd.detectChanges();
-            })
-        );
-    }
-
-    /**
-     * The type of the chart. Defaults to `'bar'`.
-     */
-    @Input()
-    public set type(type: ChartType) {
-        this._type = type;
-        this.checkAndUpdateChartType();
-        this.cd.detectChanges();
-    }
-
-    public get type(): ChartType {
-        return this._type;
-    }
-
-    @Input()
-    public set chartLegendSize(size: ChartLegendSize) {
-        this._chartLegendSize = size;
-        this.setupChartLegendSize();
-    }
-
-    /**
-     * Whether to show the legend.
-     */
-    @Input()
-    public showLegend = true;
 
     /**
      * The labels for the separated sections.
      * Each label represent one section, e.g. one year.
      */
     @Input()
-    public labels: Label[] = [];
+    public labels: Label[];
-
-    /**
-     * Sets the position of the legend.
-     * Defaults to `'top'`.
-     */
-    @Input()
-    public set legendPosition(position: Chart.PositionType) {
-        this.chartOptions.legend.position = position;
-    }
 
     /**
      * Determine, if the chart has some padding at the borders.
@@ -131,121 +62,70 @@ export class ChartsComponent extends BaseViewComponent {
     public hasPadding = true;
 
     /**
-     * Optional passing a number as percentage value for `max-width`.
+     * Show a legend
-     * Range from 1 to 100.
-     * Defaults to `100`.
      */
     @Input()
-    public set size(size: number) {
+    public legend = false;
-        if (size > 100) {
-            size = 100;
-        }
-        if (size < 1) {
-            size = 1;
-        }
-        this._size = size;
-    }
-
-    public get size(): number {
-        return this._size;
-    }
 
     /**
-     * Fires an event, when the user clicks on the chart.
+     * Required since circle charts demand SingleDataSet-Objects
      */
-    @Output()
+    public circleColors: { backgroundColor?: string[]; hoverBackgroundColor?: string[] }[];
-    public select = new EventEmitter<ChartEvent>();
-
-    /**
-     * Fires an event, when the user hovers over the chart.
-     */
-    @Output()
-    public hover = new EventEmitter<ChartEvent>();
-
-    /**
-     * Returns a string to append to the `chart-wrapper's` classes.
-     */
-    public get classes(): string {
-        return 'os-charts os-charts--' + this.size;
-    }
 
     /**
      * The general data for the chart.
      * This is only needed for `type == 'bar' || 'line'`
      */
-    public chartData: ChartData = [];
+    public chartData: ChartData;
 
-    /**
+    @Input()
-     * The data for circle-like charts, like 'doughnut' or 'pie'.
+    public set data(inputData: ChartData) {
-     */
+        this.progressInputData(inputData);
-    public circleData: number[] = [];
+    }
-
-    /**
-     * The labels for circle-like charts, like 'doughnut' or 'pie'.
-     */
-    public circleLabels: Label[] = [];
-
-    /**
-     * The colors for circle-like charts, like 'doughnut' or 'pie'.
-     */
-    public circleColors: { backgroundColor?: string[]; hoverBackgroundColor?: string[] }[] = [];
 
     /**
      * The options used for the charts.
      */
-    public chartOptions: ChartOptions = {
+    public get chartOptions(): ChartOptions {
-        responsive: true,
+        if (this.isCircle) {
-        legend: {
+            return {
-            position: 'top',
+                aspectRatio: 1,
-            labels: {}
+                legend: {
-        },
+                    position: 'left'
-        scales: {
-            xAxes: [
-                {
-                    gridLines: {
-                        drawOnChartArea: false
-                    },
-                    ticks: { beginAtZero: true, stepSize: 1 },
-                    stacked: true
                 }
-            ],
+            };
-            yAxes: [
+        } else {
-                {
+            return {
-                    gridLines: {
+                aspectRatio: 3,
-                        drawBorder: false,
+                scales: {
-                        drawOnChartArea: false,
+                    xAxes: [
-                        drawTicks: false
+                        {
-                    },
+                            gridLines: {
-                    ticks: { mirror: true, labelOffset: -20 },
+                                drawOnChartArea: false
-                    stacked: true
+                            },
+                            ticks: { beginAtZero: true, stepSize: 1 },
+                            stacked: true
+                        }
+                    ],
+                    yAxes: [
+                        {
+                            gridLines: {
+                                drawBorder: false,
+                                drawOnChartArea: false,
+                                drawTicks: false
+                            },
+                            ticks: { mirror: true, labelOffset: -20 },
+                            stacked: true
+                        }
+                    ]
                 }
-            ]
+            };
         }
-    };
+    }
 
-    /**
+    public get isCircle(): boolean {
-     * Chart option for pie and doughnut
+        return this.type === 'pie' || this.type === 'doughnut';
-     */
+    }
-    @Input()
-    public pieChartOptions: ChartOptions = {
-        responsive: true,
-        legend: {
-            position: 'left'
-        },
-        aspectRatio: 1
-    };
-
-    /**
-     * Holds the type of the chart - defaults to `bar`.
-     */
-    private _type: ChartType = 'bar';
-
-    private _chartLegendSize: ChartLegendSize = 'middle';
-
-    /**
-     * Holds the value for `max-width`.
-     */
-    private _size = 100;
 
     /**
      * Constructor.
@@ -255,46 +135,29 @@ export class ChartsComponent extends BaseViewComponent {
      * @param matSnackbar
      * @param cd
      */
-    public constructor(
+    public constructor(title: Title, translate: TranslateService, matSnackbar: MatSnackBar) {
-        title: Title,
-        protected translate: TranslateService,
-        matSnackbar: MatSnackBar,
-        private cd: ChangeDetectorRef
-    ) {
         super(title, translate, matSnackbar);
     }
 
-    private setupBar(): void {
+    private progressInputData(inputChartData: ChartData): void {
-        if (!this.chartData.every(date => date.barThickness && date.maxBarThickness)) {
+        if (this.isCircle) {
-            this.chartData = this.chartData.map(chartDate => ({
+            this.chartData = inputChartData.flatMap(chartDate => chartDate.data);
-                ...chartDate,
+            this.circleColors = [
-                barThickness: 20,
+                {
-                maxBarThickness: 48
+                    backgroundColor: inputChartData
-            }));
+                        .map(chartDate => chartDate.backgroundColor)
+                        .filter(color => !!color),
+                    hoverBackgroundColor: inputChartData
+                        .map(chartDate => chartDate.hoverBackgroundColor)
+                        .filter(color => !!color)
+                }
+            ];
+        } else {
+            this.chartData = inputChartData;
         }
-    }
 
-    private setupChartLegendSize(): void {
+        if (!this.labels) {
-        switch (this._chartLegendSize) {
+            this.labels = inputChartData.map(chartDate => chartDate.label);
-            case 'small':
-                this.chartOptions.legend.labels = Object.assign(this.chartOptions.legend.labels, {
-                    fontSize: 10,
-                    boxWidth: 20
-                });
-                break;
-            case 'middle':
-                this.chartOptions.legend.labels = {
-                    fontSize: 14,
-                    boxWidth: 40
-                };
-        }
-        this.cd.detectChanges();
-    }
-
-    private checkAndUpdateChartType(): void {
-        if (this._type === 'stackedBar') {
-            this.setupBar();
-            this._type = 'horizontalBar';
         }
     }
 }

client/src/app/shared/components/motion-poll-detail-content/motion-poll-detail-content.component.html

@@ -34,6 +34,6 @@
 
     <!-- Chart -->
     <div class="doughnut-chart" *ngIf="showChart">
-        <os-charts type="doughnut" [data]="chartData" [showLegend]="false" [hasPadding]="false"></os-charts>
+        <os-charts type="doughnut" [data]="chartData | async" [hasPadding]="false"></os-charts>
     </div>
 </div>

client/src/app/site/assignments/components/assignment-poll-detail/assignment-poll-detail.component.html

@@ -82,12 +82,9 @@
                 <os-charts
                     class="assignment-result-chart"
                     *ngIf="chartDataSubject.value"
-                    [type]="chartType"
                     [labels]="candidatesLabels"
-                    [data]="chartDataSubject"
+                    [data]="chartDataSubject | async"
                     [hasPadding]="false"
-                    [showLegend]="false"
-                    legendPosition="right"
                 ></os-charts>
             </div>
 

client/src/app/site/assignments/components/assignment-poll-detail/assignment-poll-detail.component.ts

@@ -11,7 +11,6 @@ import { AssignmentPollRepositoryService } from 'app/core/repositories/assignmen
 import { AssignmentVoteRepositoryService } from 'app/core/repositories/assignments/assignment-vote-repository.service';
 import { GroupRepositoryService } from 'app/core/repositories/users/group-repository.service';
 import { PromptService } from 'app/core/ui-services/prompt.service';
-import { ChartType } from 'app/shared/components/charts/charts.component';
 import { VoteValue } from 'app/shared/models/poll/base-vote';
 import { BasePollDetailComponent } from 'app/site/polls/components/base-poll-detail.component';
 import { PollTableData, VotingResult } from 'app/site/polls/services/poll.service';
@@ -34,10 +33,6 @@ export class AssignmentPollDetailComponent extends BasePollDetailComponent<ViewA
 
     public candidatesLabels: string[] = [];
 
-    public get chartType(): ChartType {
-        return 'stackedBar';
-    }
-
     public constructor(
         title: Title,
         translate: TranslateService,

client/src/app/site/assignments/components/assignment-poll/assignment-poll.component.html

@@ -60,12 +60,9 @@
         <div *ngIf="poll.stateHasVotes">
             <div *osPerms="'assignments.can_manage'; or: poll.isPublished">
                 <os-charts
-                    [type]="chartType"
                     [labels]="candidatesLabels"
-                    [data]="chartDataSubject"
+                    [data]="chartDataSubject | async"
                     [hasPadding]="false"
-                    [showLegend]="!poll.isMethodY"
-                    legendPosition="right"
                 ></os-charts>
             </div>
 

client/src/app/site/assignments/components/assignment-poll/assignment-poll.component.ts

@@ -8,7 +8,6 @@ import { TranslateService } from '@ngx-translate/core';
 
 import { AssignmentPollRepositoryService } from 'app/core/repositories/assignments/assignment-poll-repository.service';
 import { PromptService } from 'app/core/ui-services/prompt.service';
-import { ChartType } from 'app/shared/components/charts/charts.component';
 import { VotingPrivacyWarningComponent } from 'app/shared/components/voting-privacy-warning/voting-privacy-warning.component';
 import { infoDialogSettings } from 'app/shared/utils/dialog-settings';
 import { BasePollComponent } from 'app/site/polls/components/base-poll.component';
@@ -39,10 +38,6 @@ export class AssignmentPollComponent extends BasePollComponent<ViewAssignmentPol
         return this._poll;
     }
 
-    public get chartType(): ChartType {
-        return 'stackedBar';
-    }
-
     public candidatesLabels: string[] = [];
 
     /**

client/src/app/site/assignments/services/assignment-poll.service.ts

@@ -181,4 +181,25 @@ export class AssignmentPollService extends PollService {
         }
         return totalByBase;
     }
+
+    public getChartLabels(poll: PollData): string[] {
+        const fields = this.getPollDataFields(poll);
+        return poll.options.map(option => {
+            const votingResults = fields.map(field => {
+                const voteValue = option[field];
+                const votingKey = this.translate.instant(this.pollKeyVerbose.transform(field));
+                const resultValue = this.parsePollNumber.transform(voteValue);
+                const resultInPercent = this.getVoteValueInPercent(voteValue, poll);
+                let resultLabel = `${votingKey}: ${resultValue}`;
+
+                // 0 is a valid number in this case
+                if (resultInPercent !== null) {
+                    resultLabel += ` (${resultInPercent})`;
+                }
+                return resultLabel;
+            });
+
+            return `${option.user.short_name} · ${votingResults.join(' · ')}`;
+        });
+    }
 }

client/src/app/site/motions/modules/motion-poll/motion-poll/motion-poll.component.html

@@ -87,13 +87,7 @@
             </div>
         </div>
         <div class="doughnut-chart">
-            <os-charts
+            <os-charts *ngIf="showChart" type="doughnut" [data]="chartDataSubject | async" [hasPadding]="false">
-                *ngIf="showChart"
-                [type]="'doughnut'"
-                [data]="chartDataSubject"
-                [showLegend]="false"
-                [hasPadding]="false"
-            >
             </os-charts>
         </div>
     </div>

client/src/app/site/polls/components/base-poll-detail.component.ts

@@ -145,7 +145,7 @@ export abstract class BasePollDetailComponent<V extends ViewBasePoll, S extends
     public async pseudoanonymizePoll(): Promise<void> {
         const title = this.translate.instant('Are you sure you want to anonymize all votes? This cannot be undone.');
         if (await this.promptService.open(title)) {
-            this.repo.pseudoanonymize(this.poll).then(() => this.onPollLoaded(), this.raiseError); // votes have changed, but not the poll, so the components have to be informed about the update
+            this.repo.pseudoanonymize(this.poll).catch(this.raiseError);
         }
     }
 
@@ -156,11 +156,6 @@ export abstract class BasePollDetailComponent<V extends ViewBasePoll, S extends
         this.pollDialog.openDialog(viewPoll);
     }
 
-    /**
-     * Called after the poll has been loaded. Meant to be overwritten by subclasses who need initial access to the poll
-     */
-    protected onPollLoaded(): void {}
-
     protected onStateChanged(): void {}
 
     protected abstract hasPerms(): boolean;
@@ -205,7 +200,6 @@ export abstract class BasePollDetailComponent<V extends ViewBasePoll, S extends
                 this.repo.getViewModelObservable(params.id).subscribe(poll => {
                     if (poll) {
                         this.poll = poll;
-                        this.onPollLoaded();
                         this.createVotesData();
                         this.initChartData();
                         this.optionsLoaded.resolve();

client/src/app/site/polls/services/poll.service.ts

@@ -148,6 +148,8 @@ export interface VotingResult {
     showPercent?: boolean;
 }
 
+const PollChartBarThickness = 20;
+
 /**
  * Shared service class for polls. Used by child classes {@link MotionPollService}
  * and {@link AssignmentPollService}
@@ -186,8 +188,8 @@ export abstract class PollService {
     public constructor(
         constants: ConstantsService,
         protected translate: TranslateService,
-        private pollKeyVerbose: PollKeyVerbosePipe,
+        protected pollKeyVerbose: PollKeyVerbosePipe,
-        private parsePollNumber: ParsePollNumberPipe
+        protected parsePollNumber: ParsePollNumberPipe
     ) {
         constants
             .get<OpenSlidesSettings>('Settings')
@@ -302,14 +304,16 @@ export abstract class PollService {
                 data: this.getResultFromPoll(poll, key),
                 label: key.toUpperCase(),
                 backgroundColor: PollColor[key],
-                hoverBackgroundColor: PollColor[key]
+                hoverBackgroundColor: PollColor[key],
+                barThickness: PollChartBarThickness,
+                maxBarThickness: PollChartBarThickness
             } as ChartDate;
         });
 
         return data;
     }
 
-    private getPollDataFields(poll: PollData | ViewBasePoll): CalculablePollKey[] {
+    protected getPollDataFields(poll: PollData | ViewBasePoll): CalculablePollKey[] {
         let fields: CalculablePollKey[];
         let isAssignment: boolean;
 
@@ -344,28 +348,13 @@ export abstract class PollService {
      * Extracts yes-no-abstain such as valid, invalids and totals from Poll and PollData-Objects
      */
     private getResultFromPoll(poll: PollData, key: CalculablePollKey): number[] {
-        return poll[key] ? [poll[key]] : poll.options.map(option => option[key]);
+        let result: number[];
-    }
+        if (poll[key]) {
-
+            result = [poll[key]];
-    public getChartLabels(poll: PollData): string[] {
+        } else {
-        const fields = this.getPollDataFields(poll);
+            result = poll.options.map(option => option[key]);
-        return poll.options.map(option => {
+        }
-            const votingResults = fields.map(field => {
+        return result;
-                const voteValue = option[field];
-                const votingKey = this.translate.instant(this.pollKeyVerbose.transform(field));
-                const resultValue = this.parsePollNumber.transform(voteValue);
-                const resultInPercent = this.getVoteValueInPercent(voteValue, poll);
-                let resultLabel = `${votingKey}: ${resultValue}`;
-
-                // 0 is a valid number in this case
-                if (resultInPercent !== null) {
-                    resultLabel += ` (${resultInPercent})`;
-                }
-                return resultLabel;
-            });
-
-            return `${option.user.short_name} · ${votingResults.join(' · ')}`;
-        });
     }
 
     public isVoteDocumented(vote: number): boolean {

client/src/app/slides/assignments/assignment-poll/assignment-poll-slide.component.html

@@ -5,11 +5,9 @@
     </div>
     <div class="charts-wrapper" *ngIf="data.data.poll.state === PollState.Published">
         <os-charts
-            [type]="stackedBar"
             [labels]="pollService.getChartLabels(data.data.poll)"
-            [data]="chartDataSubject"
+            [data]="chartDataSubject | async"
             [hasPadding]="false"
-            [pieChartOptions]="options"
         ></os-charts>
     </div>
 </ng-container>