From f3f5025f677b5b62e90a338c6a120ec9abdda108 Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Tue, 18 Jun 2024 16:15:24 +0100 Subject: [PATCH 01/16] Scale chart size based on browser zoom factor --- .../carbon-estimation.component.spec.ts | 4 +-- .../carbon-estimation.component.ts | 26 ++++++++++++++----- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/app/carbon-estimation/carbon-estimation.component.spec.ts b/src/app/carbon-estimation/carbon-estimation.component.spec.ts index 793168e7..41a2a660 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.spec.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.spec.ts @@ -69,7 +69,7 @@ describe('CarbonEstimationComponent', () => { spyOn(component.chart as ChartComponent, 'updateOptions'); spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); - component.onResize(2000, 1000, 2000); + component.onResize(2000, 1000, 2000, 1); expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ chart: { height: 1500 }, // Height will be capped at a percentage of the screen height @@ -80,7 +80,7 @@ describe('CarbonEstimationComponent', () => { spyOn(component.chart as ChartComponent, 'updateOptions'); spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); - component.onResize(1000, 500, 1000); + component.onResize(1000, 500, 1000, 1); expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ chart: { height: 1000 - estimatorBaseHeight - 200 + estimatorHeights.title }, diff --git a/src/app/carbon-estimation/carbon-estimation.component.ts b/src/app/carbon-estimation/carbon-estimation.component.ts index f44f2a5b..2ca7538e 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.ts @@ -54,14 +54,21 @@ export class CarbonEstimationComponent implements OnInit, OnDestroy { } public ngOnInit(): void { - const chartHeight = this.getChartHeight(window.innerHeight, window.innerWidth, window.screen.height); + const chartHeight = this.getChartHeight( + window.innerHeight, + window.innerWidth, + window.screen.height, + window.devicePixelRatio + ); if (chartHeight > 0) { this.chartOptions.chart.height = chartHeight; } this.resizeSubscription = fromEvent(window, 'resize') .pipe(debounceTime(500)) - .subscribe(() => this.onResize(window.innerHeight, window.innerWidth, window.screen.height)); + .subscribe(() => + this.onResize(window.innerHeight, window.innerWidth, window.screen.height, window.devicePixelRatio) + ); } public ngOnDestroy(): void { @@ -70,11 +77,11 @@ export class CarbonEstimationComponent implements OnInit, OnDestroy { public onExpanded(): void { this.changeDetectorRef.detectChanges(); - this.onResize(window.innerHeight, window.innerWidth, window.screen.height); + this.onResize(window.innerHeight, window.innerWidth, window.screen.height, window.devicePixelRatio); } - onResize(innerHeight: number, innerWidth: number, screenHeight: number): void { - const chartHeight = this.getChartHeight(innerHeight, innerWidth, screenHeight); + onResize(innerHeight: number, innerWidth: number, screenHeight: number, browserZoomFactor: number): void { + const chartHeight = this.getChartHeight(innerHeight, innerWidth, screenHeight, browserZoomFactor); if (chartHeight > 0) { this.chart?.updateOptions({ chart: { height: chartHeight } }); } @@ -135,7 +142,12 @@ export class CarbonEstimationComponent implements OnInit, OnDestroy { ); } - private getChartHeight(innerHeight: number, innerWidth: number, screenHeight: number): number { + private getChartHeight( + innerHeight: number, + innerWidth: number, + screenHeight: number, + browserZoomFactor: number // 1 = 100% zoom + ): number { const expansionPanelHeight = this.detailsPanel.nativeElement.clientHeight; const calculatedHeight = this.calculateChartHeight(innerHeight, innerWidth, expansionPanelHeight); @@ -144,7 +156,7 @@ export class CarbonEstimationComponent implements OnInit, OnDestroy { // Cap chart height based on screen height to prevent issues with the chart // becoming stretched when the component is displayed in a tall iFrame - return Math.min(calculatedHeight, screenHeight * maxScreenHeightRatio); + return Math.min(calculatedHeight, screenHeight * maxScreenHeightRatio) * browserZoomFactor; } private calculateChartHeight(innerHeight: number, innerWidth: number, expansionPanelHeight: number) { From 1165f1042e9fdc1a548a4c584978d1f64dd60b0c Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Tue, 18 Jun 2024 16:37:22 +0100 Subject: [PATCH 02/16] Add test for browser zoom scaling --- .../carbon-estimation.component.spec.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/app/carbon-estimation/carbon-estimation.component.spec.ts b/src/app/carbon-estimation/carbon-estimation.component.spec.ts index 41a2a660..992bead3 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.spec.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.spec.ts @@ -87,6 +87,17 @@ describe('CarbonEstimationComponent', () => { }); }); + it('should scale chart height according to browser zoom factor', () => { + spyOn(component.chart as ChartComponent, 'updateOptions'); + spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); + + component.onResize(1500, 1000, 2000, 2); + + expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ + chart: { height: (1500 - estimatorBaseHeight - 200) * 2 }, + }); + }); + it('should call onResize when onExpansion is called', () => { spyOn(component, 'onResize'); From 8abe6a265fc32de6ee5399842902eb19cf1d1b7b Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Tue, 18 Jun 2024 16:58:57 +0100 Subject: [PATCH 03/16] Parameterise browser zoom tests --- .../carbon-estimation.component.spec.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/app/carbon-estimation/carbon-estimation.component.spec.ts b/src/app/carbon-estimation/carbon-estimation.component.spec.ts index 992bead3..1b52de3e 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.spec.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.spec.ts @@ -87,14 +87,16 @@ describe('CarbonEstimationComponent', () => { }); }); - it('should scale chart height according to browser zoom factor', () => { - spyOn(component.chart as ChartComponent, 'updateOptions'); - spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); + [0.5, 1, 2, 5].forEach(browserZoomFactor => { + it(`should scale chart height appropriately if the browser zoom factor is ${browserZoomFactor}`, () => { + spyOn(component.chart as ChartComponent, 'updateOptions'); + spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); - component.onResize(1500, 1000, 2000, 2); + component.onResize(1500, 1000, 2000, browserZoomFactor); - expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ - chart: { height: (1500 - estimatorBaseHeight - 200) * 2 }, + expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ + chart: { height: (1500 - estimatorBaseHeight - 200) * browserZoomFactor }, + }); }); }); From 0b8b9b9c26782a89d63303bb3a1ebb99e4e73e94 Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Wed, 19 Jun 2024 09:07:12 +0100 Subject: [PATCH 04/16] Update tests --- .../carbon-estimation.component.spec.ts | 49 ++++++++++++------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/src/app/carbon-estimation/carbon-estimation.component.spec.ts b/src/app/carbon-estimation/carbon-estimation.component.spec.ts index 1b52de3e..cab00d70 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.spec.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.spec.ts @@ -65,38 +65,53 @@ describe('CarbonEstimationComponent', () => { expect(component.chartOptions.chart.height).toBe(600 - estimatorBaseHeight - 200 - 100); }); - it('should recalculate chart height on window resize, for laptop screen', () => { - spyOn(component.chart as ChartComponent, 'updateOptions'); - spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); + [0.5, 1, 2, 5].forEach(browserZoomFactor => { + it(`should recalculate chart height on window resize, for laptop screen (zoom factor: ${browserZoomFactor})`, () => { + spyOn(component.chart as ChartComponent, 'updateOptions'); + spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); - component.onResize(2000, 1000, 2000, 1); + component.onResize(1500, 1000, 2000, browserZoomFactor); - expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ - chart: { height: 1500 }, // Height will be capped at a percentage of the screen height + expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ + chart: { height: (1500 - estimatorBaseHeight - 200) * browserZoomFactor }, + }); + }); + }); + + [0.5, 1, 2, 5].forEach(browserZoomFactor => { + it(`should recalculate chart height on window resize, for mobile screen (zoom factor: ${browserZoomFactor})`, () => { + spyOn(component.chart as ChartComponent, 'updateOptions'); + spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); + + component.onResize(1000, 500, 1000, browserZoomFactor); + + expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ + chart: { height: (1000 - estimatorBaseHeight - 200 + estimatorHeights.title) * browserZoomFactor }, + }); }); }); - it('should recalculate chart height on window resize, for mobile screen', () => { + it('should cap chart height as a percentage of screen height for laptop screen', () => { spyOn(component.chart as ChartComponent, 'updateOptions'); spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); - component.onResize(1000, 500, 1000, 1); + const screenHeight = 2000; + component.onResize(2000, 1000, screenHeight, 1); expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ - chart: { height: 1000 - estimatorBaseHeight - 200 + estimatorHeights.title }, + chart: { height: screenHeight * 0.75 }, }); }); - [0.5, 1, 2, 5].forEach(browserZoomFactor => { - it(`should scale chart height appropriately if the browser zoom factor is ${browserZoomFactor}`, () => { - spyOn(component.chart as ChartComponent, 'updateOptions'); - spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); + it('should cap chart height as a percentage of screen height for mobile screen', () => { + spyOn(component.chart as ChartComponent, 'updateOptions'); + spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); - component.onResize(1500, 1000, 2000, browserZoomFactor); + const screenHeight = 1000; + component.onResize(1100, 500, screenHeight, 1); - expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ - chart: { height: (1500 - estimatorBaseHeight - 200) * browserZoomFactor }, - }); + expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ + chart: { height: screenHeight * 0.75 }, }); }); From b673c3d25ce4dfe1af81a657c63eab414ad657fd Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Thu, 20 Jun 2024 11:52:21 +0100 Subject: [PATCH 05/16] Put lower bound on chart height --- .../carbon-estimation.component.ts | 45 +++++++++---------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/src/app/carbon-estimation/carbon-estimation.component.ts b/src/app/carbon-estimation/carbon-estimation.component.ts index 2ca7538e..637397c2 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.ts @@ -54,21 +54,14 @@ export class CarbonEstimationComponent implements OnInit, OnDestroy { } public ngOnInit(): void { - const chartHeight = this.getChartHeight( - window.innerHeight, - window.innerWidth, - window.screen.height, - window.devicePixelRatio - ); + const chartHeight = this.getChartHeight(window.innerHeight, window.innerWidth, window.screen.height); if (chartHeight > 0) { this.chartOptions.chart.height = chartHeight; } this.resizeSubscription = fromEvent(window, 'resize') .pipe(debounceTime(500)) - .subscribe(() => - this.onResize(window.innerHeight, window.innerWidth, window.screen.height, window.devicePixelRatio) - ); + .subscribe(() => this.onResize(window.innerHeight, window.innerWidth, window.screen.height)); } public ngOnDestroy(): void { @@ -77,11 +70,11 @@ export class CarbonEstimationComponent implements OnInit, OnDestroy { public onExpanded(): void { this.changeDetectorRef.detectChanges(); - this.onResize(window.innerHeight, window.innerWidth, window.screen.height, window.devicePixelRatio); + this.onResize(window.innerHeight, window.innerWidth, window.screen.height); } - onResize(innerHeight: number, innerWidth: number, screenHeight: number, browserZoomFactor: number): void { - const chartHeight = this.getChartHeight(innerHeight, innerWidth, screenHeight, browserZoomFactor); + onResize(innerHeight: number, innerWidth: number, screenHeight: number): void { + const chartHeight = this.getChartHeight(innerHeight, innerWidth, screenHeight); if (chartHeight > 0) { this.chart?.updateOptions({ chart: { height: chartHeight } }); } @@ -142,12 +135,7 @@ export class CarbonEstimationComponent implements OnInit, OnDestroy { ); } - private getChartHeight( - innerHeight: number, - innerWidth: number, - screenHeight: number, - browserZoomFactor: number // 1 = 100% zoom - ): number { + private getChartHeight(innerHeight: number, innerWidth: number, screenHeight: number): number { const expansionPanelHeight = this.detailsPanel.nativeElement.clientHeight; const calculatedHeight = this.calculateChartHeight(innerHeight, innerWidth, expansionPanelHeight); @@ -156,17 +144,28 @@ export class CarbonEstimationComponent implements OnInit, OnDestroy { // Cap chart height based on screen height to prevent issues with the chart // becoming stretched when the component is displayed in a tall iFrame - return Math.min(calculatedHeight, screenHeight * maxScreenHeightRatio) * browserZoomFactor; + return Math.min(calculatedHeight, screenHeight * maxScreenHeightRatio); } private calculateChartHeight(innerHeight: number, innerWidth: number, expansionPanelHeight: number) { // medium tailwind responsive design breakpoint https://tailwindcss.com/docs/responsive-design - if (innerWidth < 768) { - return innerHeight - this.estimatorBaseHeight - expansionPanelHeight + estimatorHeights.title; - } + const responsiveBreakpoint = 768; + const extraHeightString = this.extraHeight(); const extraHeight = Number(extraHeightString) || 0; - return innerHeight - this.estimatorBaseHeight - extraHeight - expansionPanelHeight; + + const calculatedHeight = + innerWidth < responsiveBreakpoint ? + innerHeight - this.estimatorBaseHeight - expansionPanelHeight + estimatorHeights.title + : innerHeight - this.estimatorBaseHeight - extraHeight - expansionPanelHeight; + + // Cap on the mininum height of the chart to prevent the chart becoming squashed when zooming + // in on desktop browsers. (Zooming in results in window.innerHeight decreasing proportionally + // on most desktop browsers which can result in the calculatedHeight above becoming too small + // or even negative. N.B. Mobile browsers behave differently.) + const minChartHeight = 300; + + return Math.max(calculatedHeight, minChartHeight); } private getDataItem(key: string, value: number, parent: string): ApexChartDataItem { From a2939b01ec2e1b82e829c303f921aa062476caaf Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Thu, 20 Jun 2024 12:08:04 +0100 Subject: [PATCH 06/16] Revert test updates --- .../carbon-estimation.component.spec.ts | 40 +++---------------- 1 file changed, 6 insertions(+), 34 deletions(-) diff --git a/src/app/carbon-estimation/carbon-estimation.component.spec.ts b/src/app/carbon-estimation/carbon-estimation.component.spec.ts index cab00d70..793168e7 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.spec.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.spec.ts @@ -65,53 +65,25 @@ describe('CarbonEstimationComponent', () => { expect(component.chartOptions.chart.height).toBe(600 - estimatorBaseHeight - 200 - 100); }); - [0.5, 1, 2, 5].forEach(browserZoomFactor => { - it(`should recalculate chart height on window resize, for laptop screen (zoom factor: ${browserZoomFactor})`, () => { - spyOn(component.chart as ChartComponent, 'updateOptions'); - spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); - - component.onResize(1500, 1000, 2000, browserZoomFactor); - - expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ - chart: { height: (1500 - estimatorBaseHeight - 200) * browserZoomFactor }, - }); - }); - }); - - [0.5, 1, 2, 5].forEach(browserZoomFactor => { - it(`should recalculate chart height on window resize, for mobile screen (zoom factor: ${browserZoomFactor})`, () => { - spyOn(component.chart as ChartComponent, 'updateOptions'); - spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); - - component.onResize(1000, 500, 1000, browserZoomFactor); - - expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ - chart: { height: (1000 - estimatorBaseHeight - 200 + estimatorHeights.title) * browserZoomFactor }, - }); - }); - }); - - it('should cap chart height as a percentage of screen height for laptop screen', () => { + it('should recalculate chart height on window resize, for laptop screen', () => { spyOn(component.chart as ChartComponent, 'updateOptions'); spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); - const screenHeight = 2000; - component.onResize(2000, 1000, screenHeight, 1); + component.onResize(2000, 1000, 2000); expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ - chart: { height: screenHeight * 0.75 }, + chart: { height: 1500 }, // Height will be capped at a percentage of the screen height }); }); - it('should cap chart height as a percentage of screen height for mobile screen', () => { + it('should recalculate chart height on window resize, for mobile screen', () => { spyOn(component.chart as ChartComponent, 'updateOptions'); spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); - const screenHeight = 1000; - component.onResize(1100, 500, screenHeight, 1); + component.onResize(1000, 500, 1000); expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ - chart: { height: screenHeight * 0.75 }, + chart: { height: 1000 - estimatorBaseHeight - 200 + estimatorHeights.title }, }); }); From d95fa5fa057c2111d76aedc49618133b2bae4ac9 Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Thu, 20 Jun 2024 12:15:23 +0100 Subject: [PATCH 07/16] Separate test cases where chart height is capped by screen height --- .../carbon-estimation.component.spec.ts | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/app/carbon-estimation/carbon-estimation.component.spec.ts b/src/app/carbon-estimation/carbon-estimation.component.spec.ts index 793168e7..c9961bb2 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.spec.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.spec.ts @@ -69,10 +69,10 @@ describe('CarbonEstimationComponent', () => { spyOn(component.chart as ChartComponent, 'updateOptions'); spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); - component.onResize(2000, 1000, 2000); + component.onResize(1500, 1000, 2000); expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ - chart: { height: 1500 }, // Height will be capped at a percentage of the screen height + chart: { height: 1500 - estimatorBaseHeight - 200 }, }); }); @@ -87,6 +87,30 @@ describe('CarbonEstimationComponent', () => { }); }); + it('should cap chart height as a percentage of screen height, for laptop screen', () => { + spyOn(component.chart as ChartComponent, 'updateOptions'); + spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); + + const screenHeight = 2000; + component.onResize(2000, 1000, screenHeight); + + expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ + chart: { height: screenHeight * 0.75 }, + }); + }); + + it('should cap chart height as a percentage of screen height, for mobile screen', () => { + spyOn(component.chart as ChartComponent, 'updateOptions'); + spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); + + const screenHeight = 1200; + component.onResize(1200, 500, screenHeight); + + expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ + chart: { height: screenHeight * 0.75 }, + }); + }); + it('should call onResize when onExpansion is called', () => { spyOn(component, 'onResize'); From 2a90b42aca190e6c37f626637409aee81c2771c3 Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Thu, 20 Jun 2024 14:45:30 +0100 Subject: [PATCH 08/16] Delete test that is no longer needed This test also seemed to be broken. I think it should have failed after the changes and it passes when the 'extraHeight' is set to '0'. --- .../carbon-estimation.component.spec.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/app/carbon-estimation/carbon-estimation.component.spec.ts b/src/app/carbon-estimation/carbon-estimation.component.spec.ts index c9961bb2..280cd422 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.spec.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.spec.ts @@ -119,17 +119,6 @@ describe('CarbonEstimationComponent', () => { expect(component.onResize).toHaveBeenCalledTimes(1); }); - it('chart updateOptions should be not be called if inner height less than base height and extra height', () => { - spyOn(component.chart as ChartComponent, 'updateOptions'); - - fixture.componentRef.setInput('extraHeight', '600'); - fixture.detectChanges(); - component.ngOnInit(); - fixture.detectChanges(); - - expect(component.chart?.updateOptions).not.toHaveBeenCalled(); - }); - it('should set emissions with total % and category breakdown', () => { const expectedEmissions = [ { From a93c77eb1092b06593b46b27d71181ae315ce61f Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Thu, 20 Jun 2024 15:00:33 +0100 Subject: [PATCH 09/16] Update remaining tests --- .../carbon-estimation.component.spec.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/app/carbon-estimation/carbon-estimation.component.spec.ts b/src/app/carbon-estimation/carbon-estimation.component.spec.ts index 280cd422..a595d481 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.spec.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.spec.ts @@ -48,21 +48,26 @@ describe('CarbonEstimationComponent', () => { }); it('should set chart height when inner height is more than base height', () => { + spyOnProperty(window, 'innerHeight').and.returnValue(1000); spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); + component.ngOnInit(); fixture.detectChanges(); - expect(component.chartOptions.chart.height).toBe(600 - estimatorBaseHeight - 200); + console.log(`window.innerHeight: ${window.innerHeight}`); + expect(component.chartOptions.chart.height).toBe(1000 - estimatorBaseHeight - 200); }); it('should set chart height to value if inner height more than base height plus extra height', () => { + spyOnProperty(window, 'innerHeight').and.returnValue(1000); spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); + fixture.componentRef.setInput('extraHeight', '100'); fixture.detectChanges(); component.ngOnInit(); fixture.detectChanges(); - expect(component.chartOptions.chart.height).toBe(600 - estimatorBaseHeight - 200 - 100); + expect(component.chartOptions.chart.height).toBe(1000 - estimatorBaseHeight - 200 - 100); }); it('should recalculate chart height on window resize, for laptop screen', () => { From 58b30d1e54a6a43a4be34b4c8aef30b29aaed3db Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Thu, 20 Jun 2024 15:18:07 +0100 Subject: [PATCH 10/16] Add test for lower bound on chart height --- .../carbon-estimation.component.spec.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/app/carbon-estimation/carbon-estimation.component.spec.ts b/src/app/carbon-estimation/carbon-estimation.component.spec.ts index a595d481..fb525430 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.spec.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.spec.ts @@ -54,7 +54,6 @@ describe('CarbonEstimationComponent', () => { component.ngOnInit(); fixture.detectChanges(); - console.log(`window.innerHeight: ${window.innerHeight}`); expect(component.chartOptions.chart.height).toBe(1000 - estimatorBaseHeight - 200); }); @@ -116,6 +115,17 @@ describe('CarbonEstimationComponent', () => { }); }); + it('should have a chart height of 300 for small innerHeight values (if screen height is large enough)', () => { + spyOn(component.chart as ChartComponent, 'updateOptions'); + spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); + + component.onResize(100, 1000, 2000); + + expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ + chart: { height: 300 }, + }); + }); + it('should call onResize when onExpansion is called', () => { spyOn(component, 'onResize'); From aab4b2b5cd3826f027b5554ef56025517f6a8ec7 Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Thu, 20 Jun 2024 15:34:08 +0100 Subject: [PATCH 11/16] Add debugging log statements --- .../carbon-estimation/carbon-estimation.component.spec.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/app/carbon-estimation/carbon-estimation.component.spec.ts b/src/app/carbon-estimation/carbon-estimation.component.spec.ts index fb525430..01f5e38b 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.spec.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.spec.ts @@ -48,12 +48,17 @@ describe('CarbonEstimationComponent', () => { }); it('should set chart height when inner height is more than base height', () => { + console.log(`window.innerHeight: ${window.innerHeight}`); spyOnProperty(window, 'innerHeight').and.returnValue(1000); spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); component.ngOnInit(); fixture.detectChanges(); + console.log(`window.innerHeight: ${window.innerHeight}`); + console.log(`screen height: ${window.screen.height}`); + console.log(`device pixel ratio: ${window.devicePixelRatio}`); + expect(component.chartOptions.chart.height).toBe(1000 - estimatorBaseHeight - 200); }); From 5e26cb95c949271312e8256757606ebd3588b7f1 Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Thu, 20 Jun 2024 16:18:02 +0100 Subject: [PATCH 12/16] Update tests for chart height on initialisation --- .../carbon-estimation.component.spec.ts | 21 +++---------------- 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/src/app/carbon-estimation/carbon-estimation.component.spec.ts b/src/app/carbon-estimation/carbon-estimation.component.spec.ts index 01f5e38b..a7c3d295 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.spec.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.spec.ts @@ -47,33 +47,18 @@ describe('CarbonEstimationComponent', () => { fixture.detectChanges(); }); - it('should set chart height when inner height is more than base height', () => { - console.log(`window.innerHeight: ${window.innerHeight}`); + it('should set the chart height when the component is initialised', () => { spyOnProperty(window, 'innerHeight').and.returnValue(1000); + spyOnProperty(window, 'innerWidth').and.returnValue(1000); + spyOnProperty(window.screen, 'height').and.returnValue(1080); spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); component.ngOnInit(); fixture.detectChanges(); - console.log(`window.innerHeight: ${window.innerHeight}`); - console.log(`screen height: ${window.screen.height}`); - console.log(`device pixel ratio: ${window.devicePixelRatio}`); - expect(component.chartOptions.chart.height).toBe(1000 - estimatorBaseHeight - 200); }); - it('should set chart height to value if inner height more than base height plus extra height', () => { - spyOnProperty(window, 'innerHeight').and.returnValue(1000); - spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); - - fixture.componentRef.setInput('extraHeight', '100'); - fixture.detectChanges(); - component.ngOnInit(); - fixture.detectChanges(); - - expect(component.chartOptions.chart.height).toBe(1000 - estimatorBaseHeight - 200 - 100); - }); - it('should recalculate chart height on window resize, for laptop screen', () => { spyOn(component.chart as ChartComponent, 'updateOptions'); spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); From 339a67a5b8dd0854f8cf4e4e6d4e82a2a87e6581 Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Thu, 20 Jun 2024 16:18:53 +0100 Subject: [PATCH 13/16] Remove height > 0 condition for setting chart height on initialisation This is no longer necessary now getChartHeight always returns a positive value. --- src/app/carbon-estimation/carbon-estimation.component.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/app/carbon-estimation/carbon-estimation.component.ts b/src/app/carbon-estimation/carbon-estimation.component.ts index 637397c2..03cc235a 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.ts @@ -75,9 +75,7 @@ export class CarbonEstimationComponent implements OnInit, OnDestroy { onResize(innerHeight: number, innerWidth: number, screenHeight: number): void { const chartHeight = this.getChartHeight(innerHeight, innerWidth, screenHeight); - if (chartHeight > 0) { - this.chart?.updateOptions({ chart: { height: chartHeight } }); - } + this.chart?.updateOptions({ chart: { height: chartHeight } }); } private getOverallEmissionPercentages(carbonEstimation: CarbonEstimation): ApexAxisChartSeries { From 219a2c2c809db17775ff489a16bc5c76bffedc2f Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Fri, 21 Jun 2024 09:04:04 +0100 Subject: [PATCH 14/16] Move all height calculations to getCHartHeight --- .../carbon-estimation.component.ts | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/app/carbon-estimation/carbon-estimation.component.ts b/src/app/carbon-estimation/carbon-estimation.component.ts index 03cc235a..274e8d74 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.ts @@ -136,16 +136,6 @@ export class CarbonEstimationComponent implements OnInit, OnDestroy { private getChartHeight(innerHeight: number, innerWidth: number, screenHeight: number): number { const expansionPanelHeight = this.detailsPanel.nativeElement.clientHeight; - const calculatedHeight = this.calculateChartHeight(innerHeight, innerWidth, expansionPanelHeight); - - const maxScreenHeightRatio = 0.75; - - // Cap chart height based on screen height to prevent issues with the chart - // becoming stretched when the component is displayed in a tall iFrame - return Math.min(calculatedHeight, screenHeight * maxScreenHeightRatio); - } - - private calculateChartHeight(innerHeight: number, innerWidth: number, expansionPanelHeight: number) { // medium tailwind responsive design breakpoint https://tailwindcss.com/docs/responsive-design const responsiveBreakpoint = 768; @@ -157,13 +147,18 @@ export class CarbonEstimationComponent implements OnInit, OnDestroy { innerHeight - this.estimatorBaseHeight - expansionPanelHeight + estimatorHeights.title : innerHeight - this.estimatorBaseHeight - extraHeight - expansionPanelHeight; - // Cap on the mininum height of the chart to prevent the chart becoming squashed when zooming - // in on desktop browsers. (Zooming in results in window.innerHeight decreasing proportionally - // on most desktop browsers which can result in the calculatedHeight above becoming too small - // or even negative. N.B. Mobile browsers behave differently.) + // Bound smallest chart height to prevent it becoming squashed when zooming + // on desktop (zooming decreases innerHeight on most desktop browsers) const minChartHeight = 300; - return Math.max(calculatedHeight, minChartHeight); + // Cap chart height based on screen height to prevent issues with the chart + // becoming stretched when the component is displayed in a tall iFrame + const maxScreenHeightRatio = 0.75; + + const heightBoundedAbove = Math.min(calculatedHeight, screenHeight * maxScreenHeightRatio); + const heightBoundedAboveAndBelow = Math.max(heightBoundedAbove, minChartHeight); + + return heightBoundedAboveAndBelow; } private getDataItem(key: string, value: number, parent: string): ApexChartDataItem { From 3c84c42ba2c7ad4d8ebd40c97587e53c99629902 Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Fri, 21 Jun 2024 11:52:03 +0100 Subject: [PATCH 15/16] Add test for extraHeight input --- .../carbon-estimation.component.spec.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/app/carbon-estimation/carbon-estimation.component.spec.ts b/src/app/carbon-estimation/carbon-estimation.component.spec.ts index a7c3d295..ac57306b 100644 --- a/src/app/carbon-estimation/carbon-estimation.component.spec.ts +++ b/src/app/carbon-estimation/carbon-estimation.component.spec.ts @@ -59,6 +59,18 @@ describe('CarbonEstimationComponent', () => { expect(component.chartOptions.chart.height).toBe(1000 - estimatorBaseHeight - 200); }); + it('should subtract the extraHeight input from the chart height on laptop screens', () => { + spyOn(component.chart as ChartComponent, 'updateOptions'); + spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); + fixture.componentRef.setInput('extraHeight', '100'); + + component.onResize(1500, 1000, 2000); + + expect(component.chart?.updateOptions).toHaveBeenCalledOnceWith({ + chart: { height: 1500 - estimatorBaseHeight - 200 - 100 }, + }); + }); + it('should recalculate chart height on window resize, for laptop screen', () => { spyOn(component.chart as ChartComponent, 'updateOptions'); spyOnProperty(component.detailsPanel.nativeElement, 'clientHeight').and.returnValue(200); From 86b4f392982bd10a53ab021e9bb4bf530d40cdfc Mon Sep 17 00:00:00 2001 From: jantoun-scottlogic Date: Fri, 21 Jun 2024 11:58:11 +0100 Subject: [PATCH 16/16] Update extra-height input documentation --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 467aa4f1..017c5089 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ The tech carbon estimator is a web component that allow you to estimate, at high exposed as a web component `tech-carbon-estimator`. The component takes the follow optional input: -- `extra-height` - this is extra height to be added whe calculating the height if the chart. eg a header/footer that will be visible on the page +- `extra-height` - this is extra height to be taken into account when calculating the height of the chart. E.g. the height of a header/footer that will be visible on the page. You will need to import the following files to use the tech-carbon-estimator: