-
-
Notifications
You must be signed in to change notification settings - Fork 69
Impact Dashboard - Cmu Practicum 23 #1075
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
Conversation
Implemented UI for 4 datapoints in homepage (Karina & Ivy)
Installed react-chart js library, implemented ROI line chart following the UI design (Karina & Ivy)
…cTechExchange into cmupracticum23
Implemented impact dashboard Hackathon section as two cards, updated some spacing set up and css styles
mplemented Impact Across Sectors, Volunteer Matching, and Volunteer Experience sections (Karina & Ivy)
Integrated 4 APIs to front end (ROI, estimated impact, number of active volunteer, number of active project) -- Karina & Ivy
…cTechExchange into cmupracticum23
…ct stringfy method
…cTechExchange into cmupracticum23
Implemented responsive UI (mobile screens) for 4 cards - Karina & Ivy
Karina & Ivy
…cTechExchange into cmupracticum23
Completed 3 APIs integrations: ROI, Volunteer Renewal, Volunteer Matching (Karina & Ivy)
Karina & Ivy
Frontend: Implemented 3 sections (ROI, Volunteer Renewal, Volunteer Matching) for mobile views with the most recent API integrations (Karina & Ivy)
Frontend: Implemented mobile views for Volunteer Roles and Hackathons (Karina & Ivy)
…ackathon Frontend: Integrated 3 APIs for Impact of Sectors, Volunteer Roles, Hackathon (Karina & Ivy)
| } | ||
|
|
||
|
|
||
| class DollarsSaved(models.Model): |
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.
TODO: Refactor impact stats into a more modular key/value store.
| ), | ||
| "project_issue_area": Tag.hydrate_to_json( | ||
| self.id, list(self.project_issue_area.all().values()) | ||
| ), # use this for project Issue area API |
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.
Extraneous comment
| role = TaggableManager(blank=True, through=TaggedVolunteerRole) # volunteer role | ||
| role.remote_field.related_name = "+" # volunteer role |
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.
Extraneous comment
| overflow: hidden; | ||
| align-self: start; // stops character descenders from getting clipped, weird but works | ||
| } | ||
| // .AggregatedDashboard-title { |
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.
Remove unused
| display: flex; | ||
| flex-direction: column; | ||
| justify-content: space-around; | ||
| // justify-content: center; |
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.
Remove unused
| import React from "react"; | ||
| import { Doughnut } from 'react-chartjs-2'; // References: https://react-chartjs-2.js.org/ | ||
|
|
||
| // 570 Business |
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.
Remove extraneous comment.
| legends[0].fillStyle = "#F79E02"; | ||
| legends[1].fillStyle = "#FDE2B3"; |
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.
These colors are defined above, use those instead or put in a central variable.
| } | ||
|
|
||
| componentDidMount(): void { | ||
| document.getElementById('detailButton').style.display = 'none'; |
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.
TODO: Inline style instead
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.
Why is this being deleted?
| @@ -1,5 +1,4 @@ | |||
| from django.apps import AppConfig | |||
|
|
|||
|
|
|||
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.
Extraneous space
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.
There are serious performance issues with this page as it stands. Multiple components have their own separate API calls, which on aggregate result in a long, fragmented page load where individual charts load at different times. For the sake of performance, I recommend putting all the data behind a single API call (whose payload should be cached since the data does not change often).
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.
Resolved in #1088
|
Closing this PR as it lives on as #1088 |
No description provided.