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

Add Stat Spark Lines & Energy Mix Pie Chart #116

Merged
merged 19 commits into from
Aug 31, 2024
Merged

Conversation

vkoves
Copy link
Owner

@vkoves vkoves commented Aug 21, 2024

Description

Added a new SparkLine component that can render a basic line graph, and added a spark line for each of the main four stat tiles (after hiding site and source EUI), as well as a new PieChart that shows the mix of energy uses:

Example from Merch Mart:

Screenshot from 2024-08-27 23-46-57

Additionally, there's a hover behavior that shows all the underlying data points, and lets you hover over them to view exact values:

Peek.2024-08-27.21-30.mp4

Resolves #108, works on #110.

Testing Instructions

Tested several buildings and confirmed the spark lines look good on desktop and mobile, with the min and max point values staying in the graph bounds. For example, here's Crown Hall (which bucks the trend of stats going down):

Screenshot from 2024-08-27 19-50-01

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link

netlify bot commented Aug 21, 2024

Deploy Preview for radiant-cucurucho-d09bae ready!

Name Link
🔨 Latest commit d38ce0a
🔍 Latest deploy log https://app.netlify.com/sites/radiant-cucurucho-d09bae/deploys/66d274956811ff000878a6d8
😎 Deploy Preview https://deploy-preview-116--radiant-cucurucho-d09bae.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

src/components/SparkLine.vue Fixed Show fixed Hide fixed

randomId = Math.round(Math.random() * 1000);

svg!: d3.Selection<SVGGElement, unknown, HTMLElement, any>;

Check warning

Code scanning / ESLint

Disallow the `any` type Warning

Unexpected any. Specify a different type.
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/StatTile.vue Fixed Show fixed Hide fixed
@vkoves vkoves changed the title Cleanup Building Details Cleanup Building Details & Add Spark Line Aug 28, 2024
const xVals: Array<number> = this.graphData.map((d) => d.x);
const yVals: Array<number> = this.graphData.map((d) => d.y);

const minYear = d3.min(xVals)!;

Check warning

Code scanning / ESLint

Disallow non-null assertions using the `!` postfix operator Warning

Forbidden non-null assertion.
const yVals: Array<number> = this.graphData.map((d) => d.y);

const minYear = d3.min(xVals)!;
const maxYear = d3.max(xVals)!;

Check warning

Code scanning / ESLint

Disallow non-null assertions using the `!` postfix operator Warning

Forbidden non-null assertion.
src/components/SparkLine.vue Fixed Show fixed Hide fixed
src/components/SparkLine.vue Fixed Show fixed Hide fixed
@vkoves vkoves added the enhancement New feature or request label Aug 28, 2024
@vkoves vkoves marked this pull request as ready for review August 28, 2024 00:52
This lets us actually investigate the underlying values

randomId = Math.round(Math.random() * 1000);

tooltip?: d3.Selection<HTMLDivElement, unknown, HTMLElement, any>;

Check warning

Code scanning / ESLint

Disallow the `any` type Warning

Unexpected any. Specify a different type.
.enter()
.append("circle")
.attr('class', (d) => {
const isMinMax = d.x === this.minAndMaxPoints![0].x

Check warning

Code scanning / ESLint

Disallow non-null assertions using the `!` postfix operator Warning

Forbidden non-null assertion.
.append("circle")
.attr('class', (d) => {
const isMinMax = d.x === this.minAndMaxPoints![0].x
|| d.x === this.minAndMaxPoints![1].x;

Check warning

Code scanning / ESLint

Disallow non-null assertions using the `!` postfix operator Warning

Forbidden non-null assertion.
// Calculate a scale factor to match the internal SVG space to the rendered HTML space
const outerWidth = this.width + this.graphMargins.left + this.graphMargins.right;
const svgElem = document.getElementById(`${this.svgIdPrefix}${this.randomId}`);
const scaleFactor = svgElem!.clientWidth / outerWidth;

Check warning

Code scanning / ESLint

Disallow non-null assertions using the `!` postfix operator Warning

Forbidden non-null assertion.
const tooltipX = d3.pointer(event)[0] * scaleFactor + 20;
const tooltipY = d3.pointer(event)[1] * scaleFactor;

this.tooltip!

Check warning

Code scanning / ESLint

Disallow non-null assertions using the `!` postfix operator Warning

Forbidden non-null assertion.
src/components/SparkLine.vue Fixed Show fixed Hide fixed

readonly graphMargins = { top: 0, right: 0, bottom: 0, left: 0 };

svg!: d3.Selection<SVGGElement, unknown, HTMLElement, any>;

Check warning

Code scanning / ESLint

Disallow the `any` type Warning

Unexpected any. Specify a different type.

// Compute the position of each group on the pie:
var pie = d3.pie()
.value((d: any) => d.value);

Check warning

Code scanning / ESLint

Disallow the `any` type Warning

Unexpected any. Specify a different type.
// Compute the position of each group on the pie:
var pie = d3.pie()
.value((d: any) => d.value);
var dataReady = pie(this.graphData as any);

Check warning

Code scanning / ESLint

Disallow the `any` type Warning

Unexpected any. Specify a different type.
.data(dataReady)
.enter()
.append('path')
.attr('d', arcGenerator as any)

Check warning

Code scanning / ESLint

Disallow the `any` type Warning

Unexpected any. Specify a different type.
return '';
}

let data = d.data as any as IPieSlice;

Check warning

Code scanning / ESLint

Disallow the `any` type Warning

Unexpected any. Specify a different type.
src/components/StatTile.vue Fixed Show fixed Hide fixed
src/components/StatTile.vue Fixed Show fixed Hide fixed
@vkoves vkoves changed the title Cleanup Building Details & Add Spark Line Add Stat Spark Lines & Energy Mix Pie Chart Aug 28, 2024
@vkoves
Copy link
Owner Author

vkoves commented Aug 28, 2024

@derekeder - would you be down to tackle review?

It is now confusing when right next to a spark line
</template>
{{ statValue }} <span
class="unit"
v-html="unit"

Check warning

Code scanning / ESLint

disallow use of v-html to prevent XSS attack Warning

'v-html' directive can lead to XSS attack.
@derekeder
Copy link
Collaborator

sure I can take a look

Copy link
Collaborator

@derekeder derekeder left a comment

Choose a reason for hiding this comment

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

overall, a great improvement. i think the pie charts are especially informative for quickly seeing the energy mix for a particular building. I clicked through about 30 different buildings and they all looked good to me.

one issue - the sparkline text is very tiny and almost impossible to read. I realize the units are whats taking the most space here. since the units are already stated in the large number in the same box, could we hide it from the sparkline and make the text bigger?

Upped font size, dropped units, and dropped decimals on large numbers
.attr("fill", "none")
.attr("stroke", "black")
.attr("stroke-width", 8)
.attr("d", (d3.line() as any)

Check warning

Code scanning / ESLint

Disallow the `any` type Warning

Unexpected any. Specify a different type.
let yPos = y(d.y);

// The min point should have its label below
if (d.x === this.minAndMaxPoints![0].x) {

Check warning

Code scanning / ESLint

Disallow non-null assertions using the `!` postfix operator Warning

Forbidden non-null assertion.
@vkoves
Copy link
Owner Author

vkoves commented Aug 30, 2024

@derekeder - updated! I'm a little torn, my chemistry teacher would always want a unit on a graph, but I think you're right that the unit is right there. We can always revise it and add it to a title:

State Image
Before Screenshot from 2024-08-30 18-32-52
After Screenshot from 2024-08-30 18-33-00

@vkoves
Copy link
Owner Author

vkoves commented Aug 31, 2024

One thing to tackle later is buildings with data gaps don't render the spark line correctly, like Willis Tower:

Screenshot from 2024-08-30 19-18-40

@vkoves
Copy link
Owner Author

vkoves commented Aug 31, 2024

Final update, I added a note to Release Notes, going to merge it in!

Screenshot from 2024-08-30 20-32-54

@vkoves vkoves merged commit d7c1767 into main Aug 31, 2024
7 checks passed
@vkoves vkoves deleted the cleanup-build-details branch August 31, 2024 01:47
@derekeder
Copy link
Collaborator

@vkoves nice! i think its a great improvement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Smaller Graphs for Each Stat Card to Show Attributes Over Time
2 participants