[feat] implements add plant details #18
Conversation
There was a problem hiding this comment.
ahhh so amazing, tysm sashank! plz see my comments
- we should merge in ur previous pr (All Plants + My Plants tab) + rebase this branch after that's merged
- there is 1 big logic fix for accessing the input values for each plants.
- let's try to add comments esp in the
movefunc to explain the logic!
| {currentIndex != plants.length + 1 && ( | ||
| <div> | ||
| <PlantDetails plant={plants[currentIndex - 1]}></PlantDetails> | ||
| <button onClick={() => move(-1)}>Back</button> |
There was a problem hiding this comment.
Design question @kyrenetam: do we want to hide (or rename) the "Back" button on the first add plant screen page? I feel like it's misleading, unless we plan to allow users to route back to view-plants
SashankBalusu
force-pushed
the
view-plant-details
branch
from
a8340cd to
4f8e04a
Compare
| plant_id: UUID; | ||
| date_added: string; | ||
| date_harvested: string; | ||
| planting_type: string; |
There was a problem hiding this comment.
this should be PlantingTypeEnum
you should also have
export type PlantingTypeEnum = 'INDOORS' | 'OUTDOORS' | 'TRANSPLANT'
| import { Plant, UserPlants } from '@/types/schema'; | ||
|
|
||
| export default function PlantDetails({ | ||
| detail, |
There was a problem hiding this comment.
this should take in setDate and setPlant
See Kyle's FilterDropdown in components/FilterDropdown.tsx for an example of how to pass in setState react hooks
| if (currentIndex != plants.length + 1) { | ||
| const updatedDetails = [...details]; | ||
| const plantID = plants[currentIndex - 1]['id']; | ||
| const date = (document.getElementById('date')! as HTMLInputElement).value; |
There was a problem hiding this comment.
instead of doing document.getElementById we can use a useState to track the dateInput and plantingType, i.e.
const [dateInput, setDateInput] = useState<Date | null>(null);
const [plantingType, setPlantingType] = useState<PlantingTypeEnum | null>(null);
(you might be able to handle the null cases differently, but this is just a suggestion)
And then, pass the value and the setValue func (e.g. plantingType, setPlantingType) into PlantDetails (see the later comment)
| ).value; | ||
| updatedDetails[currentIndex - 1] = { | ||
| date_added: date, | ||
| planting_type: plant_type, |
There was a problem hiding this comment.
this will cause a type error once we change planting_type to PlantingTypeEnum (instead of string) in schema.d.ts. Hopefully the additional value/setValue handling (see other comments) will resolve the the type error
What's new in this PR 🧑🌾
Description
Screenshots
How to review
Next steps
Relevant links
Online sources
Related PRs
CC: @ccatherinetan