From f0e396b3a4a486df33ae7d0ee69b0f209ad5056e Mon Sep 17 00:00:00 2001 From: Sean Date: Fri, 20 Mar 2020 10:28:58 +0100 Subject: [PATCH] Rework Chart component Cleans up the chart component Speed up the rendering using async pipe instead of passing obserbables Thiner bar-charts. Fixes some bugs, some bugs are still present. --- .../components/charts/charts.component.html | 39 +-- .../components/charts/charts.component.scss | 6 - .../components/charts/charts.component.ts | 273 +++++------------- .../motion-poll-detail-content.component.html | 2 +- .../assignment-poll-detail.component.html | 5 +- .../assignment-poll-detail.component.ts | 5 - .../assignment-poll.component.html | 5 +- .../assignment-poll.component.ts | 5 - .../services/assignment-poll.service.ts | 21 ++ .../motion-poll/motion-poll.component.html | 8 +- .../components/base-poll-detail.component.ts | 8 +- .../app/site/polls/services/poll.service.ts | 41 +-- .../assignment-poll-slide.component.html | 4 +- 13 files changed, 124 insertions(+), 298 deletions(-) diff --git a/client/src/app/shared/components/charts/charts.component.html b/client/src/app/shared/components/charts/charts.component.html index bf3bd225f..9d90c9240 100644 --- a/client/src/app/shared/components/charts/charts.component.html +++ b/client/src/app/shared/components/charts/charts.component.html @@ -1,26 +1,15 @@ -
- - - - - +
+ +
diff --git a/client/src/app/shared/components/charts/charts.component.scss b/client/src/app/shared/components/charts/charts.component.scss index f740fde6b..e1e0e6aee 100644 --- a/client/src/app/shared/components/charts/charts.component.scss +++ b/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 + '%'); - } -} diff --git a/client/src/app/shared/components/charts/charts.component.ts b/client/src/app/shared/components/charts/charts.component.ts index 2dab09785..dd6754404 100644 --- a/client/src/app/shared/components/charts/charts.component.ts +++ b/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'; - -/** - * Describes the events the chart is fired, when hovering or clicking on it. - */ -interface ChartEvent { - event: MouseEvent; - active: {}[]; -} +export type ChartType = 'doughnut' | 'pie' | 'horizontalBar'; /** * 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 data is prepared and splitted to dynamic use of bar/line or doughnut/pie chart. + * The type of the chart. */ @Input() - public set data(dataObservable: Observable) { - 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; + public type: ChartType = 'horizontalBar'; /** * The labels for the separated sections. * Each label represent one section, e.g. one year. */ @Input() - public labels: Label[] = []; - - /** - * Sets the position of the legend. - * Defaults to `'top'`. - */ - @Input() - public set legendPosition(position: Chart.PositionType) { - this.chartOptions.legend.position = position; - } + public labels: Label[]; /** * 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`. - * Range from 1 to 100. - * Defaults to `100`. + * Show a legend */ @Input() - public set size(size: number) { - if (size > 100) { - size = 100; - } - if (size < 1) { - size = 1; - } - this._size = size; - } - - public get size(): number { - return this._size; - } + public legend = false; /** - * Fires an event, when the user clicks on the chart. + * Required since circle charts demand SingleDataSet-Objects */ - @Output() - public select = new EventEmitter(); - - /** - * Fires an event, when the user hovers over the chart. - */ - @Output() - public hover = new EventEmitter(); - - /** - * Returns a string to append to the `chart-wrapper's` classes. - */ - public get classes(): string { - return 'os-charts os-charts--' + this.size; - } + public circleColors: { backgroundColor?: string[]; hoverBackgroundColor?: string[] }[]; /** * The general data for the chart. * This is only needed for `type == 'bar' || 'line'` */ - public chartData: ChartData = []; + public chartData: ChartData; - /** - * The data for circle-like charts, like 'doughnut' or 'pie'. - */ - 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[] }[] = []; + @Input() + public set data(inputData: ChartData) { + this.progressInputData(inputData); + } /** * The options used for the charts. */ - public chartOptions: ChartOptions = { - responsive: true, - legend: { - position: 'top', - labels: {} - }, - scales: { - xAxes: [ - { - gridLines: { - drawOnChartArea: false - }, - ticks: { beginAtZero: true, stepSize: 1 }, - stacked: true + public get chartOptions(): ChartOptions { + if (this.isCircle) { + return { + aspectRatio: 1, + legend: { + position: 'left' } - ], - yAxes: [ - { - gridLines: { - drawBorder: false, - drawOnChartArea: false, - drawTicks: false - }, - ticks: { mirror: true, labelOffset: -20 }, - stacked: true + }; + } else { + return { + aspectRatio: 3, + scales: { + xAxes: [ + { + gridLines: { + drawOnChartArea: false + }, + ticks: { beginAtZero: true, stepSize: 1 }, + stacked: true + } + ], + yAxes: [ + { + gridLines: { + drawBorder: false, + drawOnChartArea: false, + drawTicks: false + }, + ticks: { mirror: true, labelOffset: -20 }, + stacked: true + } + ] } - ] + }; } - }; + } - /** - * Chart option for pie and 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; + public get isCircle(): boolean { + return this.type === 'pie' || this.type === 'doughnut'; + } /** * Constructor. @@ -255,46 +135,29 @@ export class ChartsComponent extends BaseViewComponent { * @param matSnackbar * @param cd */ - public constructor( - title: Title, - protected translate: TranslateService, - matSnackbar: MatSnackBar, - private cd: ChangeDetectorRef - ) { + public constructor(title: Title, translate: TranslateService, matSnackbar: MatSnackBar) { super(title, translate, matSnackbar); } - private setupBar(): void { - if (!this.chartData.every(date => date.barThickness && date.maxBarThickness)) { - this.chartData = this.chartData.map(chartDate => ({ - ...chartDate, - barThickness: 20, - maxBarThickness: 48 - })); + private progressInputData(inputChartData: ChartData): void { + if (this.isCircle) { + this.chartData = inputChartData.flatMap(chartDate => chartDate.data); + this.circleColors = [ + { + 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 { - switch (this._chartLegendSize) { - 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'; + if (!this.labels) { + this.labels = inputChartData.map(chartDate => chartDate.label); } } } diff --git a/client/src/app/shared/components/motion-poll-detail-content/motion-poll-detail-content.component.html b/client/src/app/shared/components/motion-poll-detail-content/motion-poll-detail-content.component.html index 992d78bac..92c73a2a0 100644 --- a/client/src/app/shared/components/motion-poll-detail-content/motion-poll-detail-content.component.html +++ b/client/src/app/shared/components/motion-poll-detail-content/motion-poll-detail-content.component.html @@ -34,6 +34,6 @@
- +
diff --git a/client/src/app/site/assignments/components/assignment-poll-detail/assignment-poll-detail.component.html b/client/src/app/site/assignments/components/assignment-poll-detail/assignment-poll-detail.component.html index d10c18ed3..b7bf7662a 100644 --- a/client/src/app/site/assignments/components/assignment-poll-detail/assignment-poll-detail.component.html +++ b/client/src/app/site/assignments/components/assignment-poll-detail/assignment-poll-detail.component.html @@ -82,12 +82,9 @@ diff --git a/client/src/app/site/assignments/components/assignment-poll-detail/assignment-poll-detail.component.ts b/client/src/app/site/assignments/components/assignment-poll-detail/assignment-poll-detail.component.ts index 57549902b..c230f51f7 100644 --- a/client/src/app/site/assignments/components/assignment-poll-detail/assignment-poll-detail.component.ts +++ b/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
diff --git a/client/src/app/site/assignments/components/assignment-poll/assignment-poll.component.ts b/client/src/app/site/assignments/components/assignment-poll/assignment-poll.component.ts index f34baf95d..76705329f 100644 --- a/client/src/app/site/assignments/components/assignment-poll/assignment-poll.component.ts +++ b/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 { + 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(' · ')}`; + }); + } } diff --git a/client/src/app/site/motions/modules/motion-poll/motion-poll/motion-poll.component.html b/client/src/app/site/motions/modules/motion-poll/motion-poll/motion-poll.component.html index ffe9db2b6..980a48e1e 100644 --- a/client/src/app/site/motions/modules/motion-poll/motion-poll/motion-poll.component.html +++ b/client/src/app/site/motions/modules/motion-poll/motion-poll/motion-poll.component.html @@ -87,13 +87,7 @@
- +
diff --git a/client/src/app/site/polls/components/base-poll-detail.component.ts b/client/src/app/site/polls/components/base-poll-detail.component.ts index 8530fa1b5..36e41fc5e 100644 --- a/client/src/app/site/polls/components/base-poll-detail.component.ts +++ b/client/src/app/site/polls/components/base-poll-detail.component.ts @@ -145,7 +145,7 @@ export abstract class BasePollDetailComponent { 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 { if (poll) { this.poll = poll; - this.onPollLoaded(); this.createVotesData(); this.initChartData(); this.optionsLoaded.resolve(); diff --git a/client/src/app/site/polls/services/poll.service.ts b/client/src/app/site/polls/services/poll.service.ts index b347ade0c..5b36cace5 100644 --- a/client/src/app/site/polls/services/poll.service.ts +++ b/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, - private parsePollNumber: ParsePollNumberPipe + protected pollKeyVerbose: PollKeyVerbosePipe, + protected parsePollNumber: ParsePollNumberPipe ) { constants .get('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]); - } - - 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(' · ')}`; - }); + let result: number[]; + if (poll[key]) { + result = [poll[key]]; + } else { + result = poll.options.map(option => option[key]); + } + return result; } public isVoteDocumented(vote: number): boolean { diff --git a/client/src/app/slides/assignments/assignment-poll/assignment-poll-slide.component.html b/client/src/app/slides/assignments/assignment-poll/assignment-poll-slide.component.html index 656890ef0..46ee04518 100644 --- a/client/src/app/slides/assignments/assignment-poll/assignment-poll-slide.component.html +++ b/client/src/app/slides/assignments/assignment-poll/assignment-poll-slide.component.html @@ -5,11 +5,9 @@