-
Notifications
You must be signed in to change notification settings - Fork 2
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
Jai/TLI 101.e -- Added ability to upload multiple model and fixed graphs #88
Conversation
✅ Deploy Preview for cash-app-bias-busters ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
}, | ||
y: { | ||
min: 0, | ||
max: 1, | ||
beginAtZero: true, // Ensure y-axis starts at zero | ||
title: { | ||
display: true, // Show the 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.
looks good! can we do this for chart component1 as well?
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.
Yup, I already did that!
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.
great thank you!
"#99CC99", // Darker bright green 4 | ||
"#99FF99", // Bright green 5 (unchanged) | ||
"#FF5733", // Vibrant Red | ||
"#33FF57", // Vibrant Green |
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 need to change (I assume the CSS will handle that?
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.
Yea, this is just placeholder, we need to revamp the entire theme and layout of this page
@@ -188,7 +186,7 @@ const Dashboard2 = () => { | |||
|
|||
<div className="chart-section"> | |||
{Object.keys(graphData).length > 0 && ( | |||
<ChartComponent2 chartData={graphData} /> | |||
<ChartComponent2 chartData={graphData} generationalResults={generationResults} /> |
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.
can you explain what generationalResults const do?
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.
Its a variable for storing the tooltip hover data for each bar of the graph
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.
Small comments
Looks good Jai thanks! Go through my comments but other than that works fine |
Pull Request Description
Feature Implementation:
This PR enables the functionality for uploading multiple models to Dashboard 2. The system has been updated to handle file validation, properly store uploaded models, and display corresponding data and graphs for each model. The following changes have been made:
Multiple Model Upload: Users can now upload multiple models at once instead of uploading one at a time.
File Validation: The system checks the file types of uploaded models and ensures only valid model files are accepted. Invalid files trigger an error message.
Error Handling: Several error scenarios have been accounted for:
Dashboard 2 Output: The dashboard now correctly generates data and graphs for each uploaded model. Each model's respective data is shown, and graphs are plotted accordingly.
Local Storage: Uploaded models are stored in local storage and are retrievable without any issues, ensuring persistence between sessions.
Graph Enhancements:
Acceptance Criteria:
Testing: