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

117 add data visualization for reporting history #128

Merged
merged 13 commits into from
Sep 15, 2024

Conversation

noceo
Copy link
Collaborator

@noceo noceo commented Sep 9, 2024

Description

I introduced a reporting history component that shows the reporting status for every year. It starts at the year that the owners of the property reported their data for the first time and it goes until the last year of data that the website shows.
I used a list with list items that have an image indicating the status for the specific year. The images have dynamic alt texts for accessibility.
I also introduced a rating based on the number of years that were reported. At the moment it just displays the letter grade on the screen in the standard font. The layout however suggests an image/icon but I do not have an icon for every letter grade.

Please let me know whether the letter grade should stay for now or should be removed and added back in at a later time.
Also, I was not quite sure about the exact spacing between the new component and other components like the StatsTiles.

Fixes #117

Testing Instructions

I mainly did manual testing and compared the historicData property with the visual result using console logs. Some edge cases were buildings like:

1. United Center (reporting stops before the latest year of data -> therefore, adding the missing years)

Screenshot 2024-09-08 at 8 40 03 PM

2. Digital Printer's Row (missing reports in between years that are reported)

Screenshot 2024-09-08 at 10 07 23 PM

3. Searle Chemistry Laboratory (all years reported)

Screenshot 2024-09-08 at 10 07 59 PM

4. Crown Hall (only 3 years reported with the latest one missing)

Screenshot 2024-09-08 at 10 07 04 PM

Copy link

netlify bot commented Sep 9, 2024

Deploy Preview for radiant-cucurucho-d09bae ready!

Name Link
🔨 Latest commit ff3baf2
🔍 Latest deploy log https://app.netlify.com/sites/radiant-cucurucho-d09bae/deploys/66e5a0591a66790008f8a4f6
😎 Deploy Preview https://deploy-preview-128--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.

@noceo noceo requested a review from vkoves September 9, 2024 03:15
];

const score = this.reportedYearsCount / this.reportingHistory.length;
return gradeRanges.find((range) => score >= range.min)!.grade;

Check warning

Code scanning / ESLint

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

Forbidden non-null assertion.
Copy link
Owner

@vkoves vkoves left a comment

Choose a reason for hiding this comment

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

@noceo - this looks great! The letters are actually a font, and not an image, so let's use this Caveat Font from Google Fonts for that. We can then use the color #009F49 for A, #7FA52E for B buildings, #B36A15 C buildings, #972222 for D, and #FF0404 for F. I'd make sure to make that a global style (e.g. .grade-letter.-a) since it'll be used elsewhere.

Lastly I'd suggest making the icons smaller on mobile (maybe half this size?) because it's a bit large on mobile.

image


.marker {
position: relative;
width: 40px;
Copy link
Owner

Choose a reason for hiding this comment

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

Let's convert this to rem, should be 2.5rem since we're using the default 16px font size


import { LatestDataYear } from "../constants/globals.vue";

@Component
Copy link
Owner

Choose a reason for hiding this comment

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

Can you add a quick doc block, and note some good examples to test this with? We'll be writing some unit and E2E tests later, and that'll be helpful.

@vkoves vkoves added the enhancement New feature or request label Sep 10, 2024
* 3. Searle Chemistry Laboratory (all years reported)
* 4. Crown Hall (only 3 years reported with the latest one missing)
*/
@Component<any>({

Check warning

Code scanning / ESLint

Disallow the `any` type Warning

Unexpected any. Specify a different type.
Copy link
Owner

@vkoves vkoves left a comment

Choose a reason for hiding this comment

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

This looks perfect, Paul, awesome work on this! I re-tested on desktop and mobile with several buildings and it's all looking great 🎉

@vkoves vkoves merged commit b6a2569 into main Sep 15, 2024
7 checks passed
@vkoves vkoves deleted the 117-add-data-visualization-for-reporting-history branch September 15, 2024 17:22
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 Data Visualization for Reporting History
2 participants