-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add donut chart pattern #200
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
83db8e8
feat(donut): add donut chart pattern
jiekang 6f759d8
refactor(donut): update donut chart example
jiekang 8d09d21
chore(donut): fix order of imports and declarations to be alphabetical
jiekang 7f9fe68
refactor(donut): fix update implementation and manage chart loaded su…
jiekang e950066
temp(donut): temporarily refactor example to demonstrate dynamic data…
jiekang 7711c39
fix(donut): use center label text instead of function and update donu…
jiekang d33d2a9
fix(donut): add donut specific tooltip
jiekang 7fee6a8
refactor(donut): add more tests for donut chart
jiekang 6286731
refactor(donut): use unique chart id built from config.chartId
jiekang File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
import { ChartConfig } from '../chart-config'; | ||
/** | ||
* A config containing properties for the sparkline chart | ||
*/ | ||
export class DonutConfig extends ChartConfig { | ||
|
||
/** | ||
* Text for the donut chart center label (optional) | ||
*/ | ||
centerLabel?: any; | ||
|
||
/** | ||
* An optional function to handle when donut is clicked | ||
*/ | ||
onClickFn?: any; | ||
|
||
/** | ||
* The height of the donut chart (optional) | ||
*/ | ||
chartHeight?: number; | ||
|
||
/** | ||
* C3 inherited configuration for colors | ||
* An object with key-value pairs of data name and color, e.g. | ||
* colors : { | ||
* Cats: '#0088ce', | ||
* Hamsters: '#3f9c35', | ||
* } | ||
* | ||
*/ | ||
colors?: any; | ||
|
||
/** | ||
* C3 inherited configuration for size | ||
*/ | ||
size?: any; | ||
|
||
/** | ||
* C3 inherited donut configuration | ||
*/ | ||
donut?: any; | ||
|
||
/** | ||
* C3 inherited configuration for tooltip | ||
*/ | ||
tooltip?: any; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
<div #chartElement id="{{config.chartId}}"></div> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
import { | ||
async, | ||
ComponentFixture, | ||
TestBed | ||
} from '@angular/core/testing'; | ||
import { BrowserAnimationsModule } from '@angular/platform-browser/animations'; | ||
import { FormsModule } from '@angular/forms'; | ||
import { By } from '@angular/platform-browser'; | ||
import { DonutConfig } from './donut-config'; | ||
import { DonutComponent } from './donut.component'; | ||
import { ChartDefaults } from '../chart.defaults'; | ||
|
||
describe('Component: donut chart', () => { | ||
|
||
let comp: DonutComponent; | ||
let fixture: ComponentFixture<DonutComponent>; | ||
|
||
let config: DonutConfig; | ||
let data: any; | ||
|
||
beforeEach(() => { | ||
config = { | ||
chartId: 'testDonutChart', | ||
onClickFn: function(d: any, e: any) { | ||
}, | ||
data: {}, | ||
donut: { | ||
title: 'Animals' | ||
} | ||
}; | ||
data = [ | ||
['Cats', 2], | ||
['Hamsters', 2], | ||
['Dogs', 2] | ||
]; | ||
}); | ||
|
||
|
||
beforeEach(async(() => { | ||
TestBed.configureTestingModule({ | ||
imports: [BrowserAnimationsModule, FormsModule], | ||
declarations: [DonutComponent], | ||
providers: [ChartDefaults] | ||
}) | ||
.compileComponents() | ||
.then(() => { | ||
fixture = TestBed.createComponent(DonutComponent); | ||
comp = fixture.componentInstance; | ||
comp.config = config; | ||
comp.chartData = data; | ||
fixture.detectChanges(); | ||
}); | ||
})); | ||
|
||
it('should set chart id', () => { | ||
expect(comp.config.chartId).toContain('testDonutChart'); | ||
}); | ||
|
||
|
||
it('should allow attribute specification of chart height', () => { | ||
config.chartHeight = 120; | ||
fixture.detectChanges(); | ||
expect(comp.config.size.height).toBe(120); | ||
}); | ||
|
||
it('should update when the chart height attribute changes', () => { | ||
config.chartHeight = 120; | ||
|
||
fixture.detectChanges(); | ||
expect(comp.config.size.height).toBe(120); | ||
|
||
config.chartHeight = 100; | ||
fixture.detectChanges(); | ||
expect(comp.config.size.height).toBe(100); | ||
}); | ||
|
||
it('should setup C3 chart data correctly', () => { | ||
expect(comp.config.data.columns.length).toBe(3); | ||
expect(comp.config.data.columns[0][0]).toBe('Cats'); | ||
expect(comp.config.data.columns[1][0]).toBe('Hamsters'); | ||
}); | ||
|
||
it('should update C3 chart data when data changes', () => { | ||
expect(comp.config.data.columns.length).toBe(3); | ||
expect(comp.config.data.columns[0][0]).toBe('Cats'); | ||
expect(comp.config.data.columns[0][1]).toBe(2); | ||
|
||
data[0][1] = 3; | ||
fixture.detectChanges(); | ||
|
||
expect(comp.config.data.columns[0][1]).toBe(3); | ||
}); | ||
|
||
it('should setup onclick correctly', () => { | ||
expect(typeof(comp.config.data.onclick)).toBe('function'); | ||
}); | ||
|
||
it('should use the default centerLabel', () => { | ||
let centerLabel = comp.getCenterLabelText(); | ||
expect(centerLabel.bigText).toBe(6); | ||
expect(centerLabel.smText).toBe('Animals'); | ||
}); | ||
|
||
it('should use custom centerLabel', () => { | ||
config.centerLabel = 'custom-label'; | ||
fixture.detectChanges(); | ||
|
||
let centerLabel = comp.getCenterLabelText(); | ||
expect(centerLabel.bigText).toBe('custom-label'); | ||
expect(centerLabel.smText).toBe(''); | ||
}); | ||
|
||
it('should use patternfly tooltip', () => { | ||
expect(typeof(comp.config.tooltip.contents)).toBe('function'); | ||
}); | ||
|
||
it('should have default donut config with custom title', () => { | ||
expect(comp.config.donut.title).toBe('Animals'); | ||
|
||
expect(comp.config.donut.width).toBe(11); | ||
expect(comp.config.donut.label.show).toBe(false); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,148 @@ | ||
import { Component, DoCheck, Input, OnInit } from '@angular/core'; | ||
|
||
import { cloneDeep, defaults, isEqual, merge, uniqueId } from 'lodash'; | ||
import { Subscription } from 'rxjs/Subscription'; | ||
|
||
import { ChartDefaults } from '../chart.defaults'; | ||
import { ChartBase } from '../chart.base'; | ||
import { DonutConfig } from './donut-config'; | ||
|
||
import * as d3 from 'd3'; | ||
|
||
@Component({ | ||
selector: 'pfng-chart-donut', | ||
templateUrl: './donut.component.html' | ||
}) | ||
export class DonutComponent extends ChartBase implements DoCheck, OnInit { | ||
|
||
@Input() chartData: any; | ||
@Input() config: DonutConfig; | ||
|
||
private prevChartData: any; | ||
|
||
private subscriptions: Subscription[] = []; | ||
|
||
/** | ||
* Default constructor | ||
* @param chartDefaults | ||
*/ | ||
constructor(private chartDefaults: ChartDefaults) { | ||
super(); | ||
} | ||
|
||
ngOnInit(): void { | ||
if (!this.config.chartId) { | ||
throw new Error('DonutComponent: config must have string property chartId'); | ||
} | ||
this.config.chartId = uniqueId(this.config.chartId); | ||
|
||
this.subscriptions.push(this.chartLoaded.subscribe({ | ||
next: (chart: any) => { | ||
this.chartAvailable(chart); | ||
} | ||
})); | ||
|
||
this.setupConfigDefaults(); | ||
} | ||
|
||
ngDoCheck(): void { | ||
if (!isEqual(this.config, this.prevConfig)) { | ||
this.updateConfig(); | ||
this.generateChart(this.config.chartId, true); | ||
} else if (!isEqual(this.chartData, this.prevChartData)) { | ||
this.config.data = merge(this.config.data, this.getDonutData(this.chartData)); | ||
this.generateChart(this.config.chartId, false); | ||
this.prevChartData = cloneDeep(this.chartData); | ||
} | ||
} | ||
|
||
ngOnDestroy(): void { | ||
this.subscriptions.forEach(sub => sub.unsubscribe); | ||
} | ||
|
||
// Public for testing | ||
public getCenterLabelText(): any { | ||
let centerLabelText = { | ||
bigText: this.getTotal(), | ||
smText: this.config.donut.title | ||
}; | ||
|
||
if (this.config.centerLabel) { | ||
centerLabelText.bigText = this.config.centerLabel; | ||
centerLabelText.smText = ''; | ||
} | ||
|
||
return centerLabelText; | ||
} | ||
|
||
private chartAvailable(chart: any): void { | ||
this.setupDonutChartTitle(chart); | ||
} | ||
|
||
private getTotal(): number { | ||
let total = 0; | ||
this.chartData.forEach((element: any) => { | ||
if (!isNaN(element[1])) { | ||
total += Number(element[1]); | ||
} | ||
}); | ||
|
||
return total; | ||
} | ||
|
||
private setupDonutChartTitle(chart: any): void { | ||
let donutChartTitle, centerLabelText; | ||
|
||
if (!chart) { | ||
return; | ||
} | ||
|
||
donutChartTitle = d3.select(chart.element).select('text.c3-chart-arcs-title'); | ||
if (!donutChartTitle) { | ||
return; | ||
} | ||
|
||
centerLabelText = this.getCenterLabelText(); | ||
|
||
donutChartTitle.text(''); | ||
if (centerLabelText.bigText && !centerLabelText.smText) { | ||
donutChartTitle.text(centerLabelText.bigText); | ||
} else { | ||
donutChartTitle.insert('tspan', null).text(centerLabelText.bigText) | ||
.classed('donut-title-big-pf', true).attr('dy', 0).attr('x', 0); | ||
donutChartTitle.insert('tspan', null).text(centerLabelText.smText). | ||
classed('donut-title-small-pf', true).attr('dy', 20).attr('x', 0); | ||
} | ||
} | ||
|
||
private getDonutData(chartData: any): any { | ||
return { | ||
type: 'donut', | ||
columns: this.chartData, | ||
order: null, | ||
colors: this.config.colors | ||
}; | ||
} | ||
|
||
private setupConfigDefaults(): void { | ||
let defaultConfig = this.chartDefaults.getDefaultDonutConfig(); | ||
let defaultDonut = this.chartDefaults.getDefaultDonut(); | ||
|
||
defaults(this.config, defaultConfig); | ||
defaults(this.config.donut, defaultDonut); | ||
} | ||
|
||
private updateConfig(): void { | ||
this.config.data = merge(this.config.data, this.getDonutData(this.chartData)); | ||
|
||
if (this.config.chartHeight) { | ||
this.config.size.height = this.config.chartHeight; | ||
} | ||
|
||
if (this.config.onClickFn) { | ||
this.config.data.onclick = this.config.onClickFn; | ||
} | ||
|
||
this.config.tooltip = { contents: (window as any).patternfly.pfDonutTooltipContents }; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
<div class="padding-15"> | ||
<div class="row padding-bottom-15"> | ||
<div class="col-xs-12"> | ||
<h4>Donut Chart Example</h4> | ||
<hr/> | ||
</div> | ||
</div> | ||
<div class="row"> | ||
<div class="col-md-6 text-center"> | ||
<label>Donut Chart</label> | ||
</div> | ||
<div class="col-md-6 text-center"> | ||
<label>Small Donut Chart</label> | ||
</div> | ||
</div> | ||
<div class="row"> | ||
<div class="col-md-6 text-center"> | ||
<pfng-chart-donut [config]="config" [chartData]="data"></pfng-chart-donut> | ||
</div> | ||
<div class="col-md-6 text-center"> | ||
<pfng-chart-donut [config]="config2" [chartData]="data"></pfng-chart-donut> | ||
</div> | ||
</div> | ||
|
||
<div class="row padding-bottom-15 padding-top-15"> | ||
<div class="col-xs-12"> | ||
<h4>Code</h4> | ||
<hr/> | ||
</div> | ||
</div> | ||
<div> | ||
<tabset> | ||
<tab heading="api"> | ||
<iframe class="demoframe" src="docs/classes/donutcomponent.html"></iframe> | ||
</tab> | ||
<tab heading="html"> | ||
<include-content src="src/app/chart/donut/examples/donut-example.component.html"></include-content> | ||
</tab> | ||
<tab heading="typescript"> | ||
<include-content src="src/app/chart/donut/examples/donut-example.component.ts"></include-content> | ||
</tab> | ||
</tabset> | ||
</div> | ||
</div> |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of a callback function, we would like to have an onClick event in the component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This onClickFn mirrors
angular-patternfly
and would be an API difference betweenangular-patternfly
andpatternfly-ng
if changed, is that okay? My approach was to keep things one to one such that anyone deciding to port from one to the other would not have to make any config changes due to incompatibilities. However I realizeangularjs
andangular
are completely different projects so this might not be a concern for this project.I don't understand what an "onClick event in the component" means, could you elaborate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's ok to be different from APF. This is our opportunity to clean things up.
Instead of calling a function when the donut is clicked, we emit an event. A developer can easily subscript to the event like so:
<pfng-donut-chart (onClick)="myFunction($event)"...
Then, in the component we would have something like this:
/**
* The event emitted when an item has been clicked
*/
@output('onClick') onClick = new EventEmitter();
And some method to handle the click:
handleClick($event: MouseEvent): void {
this.onClick.emit({
item: item
} as ListEvent);
}