Skip to content
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

Refactor median-chart #1199

Merged
merged 6 commits into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/app/features/charts/charts.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
BoxPlotComponent,
CandlestickChartComponent,
MedianChartComponent,
MedianBarChartComponent,
RowChartComponent,
ScoreChartComponent,
ScoreBarChartComponent,
Expand All @@ -19,6 +20,7 @@ import {
BoxPlotComponent,
CandlestickChartComponent,
MedianChartComponent,
MedianBarChartComponent,
RowChartComponent,
ScoreChartComponent,
ScoreBarChartComponent,
Expand All @@ -30,6 +32,7 @@ import {
BoxPlotComponent,
CandlestickChartComponent,
MedianChartComponent,
MedianBarChartComponent,
RowChartComponent,
ScoreChartComponent,
ScoreBarChartComponent,
Expand Down
1 change: 1 addition & 0 deletions src/app/features/charts/components/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export * from './box-plot-chart';
export * from './candlestick-chart';
export * from './median-chart';
export * from './median-barchart';
export * from './row-chart';
export * from './score-chart';
export * from './score-barchart';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './median-barchart.component';
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<div id="median-barchart" #medianBarChartContainer class="chart-d3">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add #medianBarChartContainer to get div size on window resize

<div
id="tooltip"
class="arrow-above tooltip-arrow"
#tooltip
(mouseout)="hideTooltip()"
></div>
<svg #chart></svg>
<div class="chart-no-data" *ngIf="data.length === 0">
<div class="chart-no-data-text">No data is currently available.</div>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#median-barchart {
.x-axis-label, .y-axis-label {
font-size: var(--font-size-lg);
font-weight: 700;
color: var(--color-chart-axis-label);
fill: var(--color-chart-axis-label);
}

.y-axis .tick {
text {
font-size: var(--font-size-sm);
}
}

.x-axis .tick {
text {
font-size: var(--font-size-md);
font-weight: 700;
}
}

.bar-labels {
font-size: var(--font-size-sm);
text-anchor: middle;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
// -------------------------------------------------------------------------- //
// External
// -------------------------------------------------------------------------- //
import { TestBed, ComponentFixture, waitForAsync } from '@angular/core/testing';
import { RouterTestingModule } from '@angular/router/testing';

// -------------------------------------------------------------------------- //
// Internal
// -------------------------------------------------------------------------- //
import { MedianBarChartComponent } from './';
import { HelperService } from '../../../../core/services';
import { geneMock1 } from '../../../../testing';
import { MedianExpression } from '../../../../models/genes';

// -------------------------------------------------------------------------- //
// Tests
// -------------------------------------------------------------------------- //
const XAXIS_LABEL = 'BRAIN REGION';
const YAXIS_LABEL = 'LOG2CPM';
const FULL_MOCK_DATA = geneMock1.median_expression;
const TISSUES = ['TCX', 'PHG', 'STG'];
const SMALL_MOCK_DATA = [
{
min: -2.54173091337051,
first_quartile: -1.03358030635935,
median: 0.483801733963266,
mean: -0.517398199667356,
third_quartile: -0.0800759845652829,
max: 2.32290808871289,
tissue: TISSUES[0],
},
{
min: -2.44077907413711,
first_quartile: -0.592671867557559,
median: 0.013739530502129,
mean: -0.0324143336865,
third_quartile: 0.49577213202412,
max: 2.23019575245731,
tissue: TISSUES[1],
},
{
min: -5.03189866356294,
first_quartile: -1.02644563959975,
median: 0.176348063122062,
mean: -0.323038107200895,
third_quartile: 0.391874711168331,
max: 1.9113258251877,
tissue: TISSUES[2],
},
];

describe('Component: BarChart - Median', () => {
let fixture: ComponentFixture<MedianBarChartComponent>;
let component: MedianBarChartComponent;
let element: HTMLElement;

beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
declarations: [MedianBarChartComponent],
imports: [RouterTestingModule],
providers: [HelperService],
}).compileComponents();
}));

beforeEach(() => {
fixture = TestBed.createComponent(MedianBarChartComponent);
component = fixture.componentInstance;
});

const setUp = (
data: MedianExpression[] = FULL_MOCK_DATA,
xAxisLabel: string = XAXIS_LABEL,
yAxisLabel: string = YAXIS_LABEL
) => {
component.data = data;
component.xAxisLabel = xAxisLabel;
component.yAxisLabel = yAxisLabel;

fixture.detectChanges();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The chart is created in ngOnChanges, which is called in unit tests after fixture.detectChanges is called the first time. Otherwise, ngOnChanges isn't called when setting an input manually. Move fixture.detectChanges into setUp method, so that the correct data for the test is set when ngOnChanges is called.

element = fixture.nativeElement;
const chart = element.querySelector('svg g');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the svg exists in the template and so may be present when the chart hasn't been created, use the first child added in createChart to check whether the chart exists

return { chart };
};

it('should create', () => {
setUp();
expect(component).toBeTruthy();
});

it('should not render the chart if there is no data', () => {
const { chart } = setUp([]);

expect(component.data?.length).toEqual(0);
expect(element.querySelector('.chart-no-data')).toBeTruthy();

expect(chart).toBeFalsy();
});
Comment on lines +90 to +97
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

develop feature
main_ENSG00000234956_noData feature_ENSG00000234956_noData


it('should not render the chart if all values are negative', () => {
const { chart } = setUp(
SMALL_MOCK_DATA.map((obj) => {
return { ...obj, median: -1 * obj.median };
})
);

expect(element.querySelector('.chart-no-data')).toBeTruthy();

expect(chart).toBeFalsy();
});

it('should render the chart if there is positive data', () => {
const createChartSpy = spyOn(component, 'createChart').and.callThrough();
const { chart } = setUp();

expect(component.data?.length).toEqual(FULL_MOCK_DATA.length);
expect(createChartSpy).toHaveBeenCalled();
expect(chart).toBeTruthy();

expect(element.querySelector('.chart-no-data')).toBeFalsy();
});
Comment on lines +111 to +120
Copy link
Contributor Author

@hallieswan hallieswan Jun 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Case where medianlogcpm values are positive --

develop feature
main_ENSG00000100376_largerValues feature_ENSG00000100376_largerValues


it('should render the chart axes', () => {
const { chart } = setUp();

expect(chart?.querySelector('.x-axis')).toBeTruthy();
expect(chart?.querySelector('.y-axis')).toBeTruthy();
});

it('should render the correct number of bars', () => {
const { chart } = setUp();

expect(chart?.querySelectorAll('rect').length).toEqual(
FULL_MOCK_DATA.length
);
});

it('should not render bars for negative values', () => {
const { chart } = setUp([
{ ...SMALL_MOCK_DATA[0], median: -0.01 },
...SMALL_MOCK_DATA.slice(1),
]);

expect(chart?.querySelectorAll('rect').length).toEqual(
SMALL_MOCK_DATA.length - 1
);
});

it('should render the bar labels', () => {
const { chart } = setUp();

expect(chart?.querySelectorAll('text.bar-labels').length).toEqual(
FULL_MOCK_DATA.length
);
});

it('should render the labels for both axes', () => {
const { chart } = setUp();

expect(chart?.querySelector('.x-axis-label')?.textContent).toEqual(
XAXIS_LABEL
);
expect(chart?.querySelector('.y-axis-label')?.textContent).toEqual(
YAXIS_LABEL
);
});

it('should render the meaningful expression threshold when values are larger than threshold', () => {
const { chart } = setUp();

expect(
chart?.querySelector('.meaningful-expression-threshold-line')
).toBeTruthy();
});

it('should not render the meaningful expression threshold when all values are smaller than threshold', () => {
const { chart } = setUp(SMALL_MOCK_DATA);

expect(
chart?.querySelector('.meaningful-expression-threshold-line')
).toBeFalsy();
});

it('should alphabetize the x-axis values', () => {
setUp(SMALL_MOCK_DATA);

const sortedTissues = TISSUES.sort();
const xAxisTicks = element
.querySelector('svg g .x-axis')
?.querySelectorAll('.tick');
xAxisTicks?.forEach((val, index) => {
expect(val.textContent).toEqual(sortedTissues[index]);
});
});
Comment on lines +183 to +193
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure new implementation matches current behavior, which alphabetizes the x-axis values


it('should show and hide tooltip', () => {
setUp();

const tooltip = element.querySelector<HTMLElement>('#tooltip');
expect(tooltip?.textContent).toBeFalsy();

const xAxisTick = element.querySelector('svg g .x-axis .tick');
const mouseEnterEvent = new MouseEvent('mouseenter', {
bubbles: true,
cancelable: true,
});
xAxisTick?.dispatchEvent(mouseEnterEvent);

expect(tooltip?.style.display).toEqual('block');
expect(tooltip?.textContent).toBeTruthy();

const mouseLeaveEvent = new MouseEvent('mouseleave', {
bubbles: true,
cancelable: true,
});
xAxisTick?.dispatchEvent(mouseLeaveEvent);

expect(tooltip?.style.display).toEqual('none');
});
});
Comment on lines +195 to +219
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

develop

develop_tooltips.mov

feature

feature_tooltips.mov

Loading
Loading