[CODE] add csv download#90
Conversation
There was a problem hiding this comment.
Fantastic first pull request, great job!! π₯³ I left some comments on what to change. Here's the TLDR:
-
Great job using reusable components (DownloadIcon, tooltips)
-
Love the working CSV downloads
-
We should try to use the Zustand store for saving state instead of using the react hook. Look at the
rxnDrawerOpenandsetRxnDrawerOpenproperties for a reference on how to use the store to manage this state. -
Let's move the styling for the button into
Simulation.css. We can keep the styling for the background color in theSimulationDrawer.tsxsince it's dynamic. -
Let's change the "green" style color to "#87F5A6"
Again, this is an amazing first request! Thank you so much for contributing to this project, this is a super valuable feature, and your turnover time was crazy quick!! Let's make these changes on THIS pr, and we'll be able to push to main. β€οΈ
| } from '@react-spring/web'; | ||
|
|
||
| import React from 'react'; | ||
| import React, { useState } from 'react'; |
There was a problem hiding this comment.
Let's try not to use state, and use our Zustand store instead!
| import { | ||
| ChevronDownIcon | ||
| ChevronDownIcon, | ||
| DownloadIcon |
There was a problem hiding this comment.
Nice, love the use of the radix components!
|
|
||
| import '../styles/index.css'; | ||
| import '../styles/Simulation.css'; | ||
| import { TooltipContent, TooltipRoot, TooltipTrigger } from './Tooltips'; |
There was a problem hiding this comment.
Chefs kiss using our reusable tooltip components!! Great work!
| from: { rotate: 0, y: '0px' }, | ||
| })); | ||
|
|
||
| const [showCsvDownloadButton, setShowCsvDownloadButton] = useState(false); |
There was a problem hiding this comment.
Understandable why you would use state, but let's try to shift to using our Zustand store
| onSimulate(); // Outer call to run simulation | ||
| } | ||
|
|
||
| const handleCSVDownload = (event: React.MouseEvent<HTMLButtonElement>) => { |
There was a problem hiding this comment.
This data looks really good, excellent job!
| <TooltipTrigger> | ||
| <button | ||
| style={{ | ||
| position: "absolute", |
There was a problem hiding this comment.
Let's move this to our styles/Simulation.css and create a DownloadButton style. We can keep the styling for the backgroundColor here though since it's dynamic!
| gap: "4px", | ||
| padding: "4px 8px", | ||
| ...(simStatus === 2 | ||
| ? { backgroundColor: "green" } |
There was a problem hiding this comment.
Let's change this to #87F5A6 to fit with existing palette
|
Thank you for the review! I will work on the fixes now. |
|
Thank you for the review! I have addressed the feedback and made the following updates:
Please take another look when you have a chance. Thanks! |
MarkAStevens04
left a comment
There was a problem hiding this comment.
Fantastic job!! Thanks again for making those changes, this looks perfect and I'll merge to main! π₯³π₯³
| const data = useStore((store) => store.simulationData); | ||
| const speciesInfo = useStore((store) => store.visualNodes); | ||
|
|
||
| const showCsvDownloadButton = useStore((store) => store.showCsvDownloadButton); |
There was a problem hiding this comment.
Fantastic, thanks for using the store!
| -moz-osx-font-smoothing: grayscale; | ||
| } | ||
|
|
||
| .DownloadButton { |
There was a problem hiding this comment.
Thanks for putting these styles in here!
|
β Thank you SO MUCH for contributing to the project!! π One of the maintainers will get back to you ASAP with some feedback. π |
Closes #72
π€ Section A - Description
This PR adds CSV download support for simulation results in
SimulationDrawer.What changed
Download CSVbutton with a download icon to the simulation drawer.simulationStatus === 2and gray otherwise.simulationDatainto CSV output using species labels derived from the existingkeyToLabelmapping.Why
Anything reviewers should know
src/react-app/components/SimulationDrawer.tsx.β Verification
[x] I ran this code locally and it does what the description says
π Quick Self Check
π When we request changes
We review to make the code better, not to nitpick. We'd also love to help you grow as a contributor! π± When we leave comments:
Thanks again for contributing, we're glad you're here!! π
π BioBuilder Contributor License Agreement
I give Mark Stevens and BioBuilder permission to license my contributions on any terms they like. I am giving them this license in order to make it possible for them to accept my contributions into their project.
As far as the law allows, my contributions come as is, without any warranty or condition, and I will not be liable to anyone for any damages related to this software or this license, under any kind of legal claim.