User/johnnytran/basictraining #328
Conversation
| const calculateRange = () => { | ||
| return; | ||
| const [speed,setSpeed] = useState(null); | ||
| const [batteryPercent, changeBatteryPercent] = useState(0); |
There was a problem hiding this comment.
Consistent naming, it should be setBatteryPercent instead of changeBatteryPercent
| const[speedValid,setSpeedValid]= useState(false); | ||
| const[batteryValid,setBatteryValid]= useState(false); | ||
| const [calc, setCalc] = useState(0) | ||
| const [initialCalc, setInitialCalc] =useState(false); | ||
|
|
||
| const calculateRange = (e) => { | ||
| e.preventDefault(); | ||
| setCalc(-(speed * speed * batteryPercent / 2500) + (4 * batteryPercent) + weather); | ||
| setInitialCalc(true); | ||
| } | ||
| const speedChangeHandler = (e) => { | ||
| if(e.target.value > 90 || e.target.value < 0){ | ||
| setSpeedValid(false) | ||
| } | ||
| else{ | ||
| setSpeedValid(true) | ||
| setSpeed(e.target.value) | ||
| console.log(speed) | ||
| } | ||
| } | ||
| const weatherChangeHandler = (e) => { | ||
| weatherChange(e.target.value); | ||
| console.log(weather) | ||
| } | ||
| const batterChangeHandler = (e) => { | ||
| if(e.target.value > 100 || e.target.value<0) { | ||
| setBatteryValid(false); | ||
| } | ||
| else{ | ||
| setBatteryValid(true); | ||
| changeBatteryPercent(e.target.value); | ||
| console.log("Battery %" + batteryPercent)} | ||
|
|
||
| } |
There was a problem hiding this comment.
Can you think of a way of handling the valid/invalid state of the speed and battery without a separate state variable?
Also, do we need to store the range in a state variable as well? Can we derive the value without using state?
| return; | ||
| const [speed,setSpeed] = useState(null); | ||
| const [batteryPercent, changeBatteryPercent] = useState(0); | ||
| const [weather, weatherChange] = useState(0); |
There was a problem hiding this comment.
Consistent naming, it should be setWeather instead of weatherChange
There was a problem hiding this comment.
Package-lock is only when you use npm, did you use npm instead of yarn by accident?
| const [calc, setCalc] = useState(0) | ||
| const [initialCalc, setInitialCalc] =useState(false); | ||
|
|
||
| const calculateRange = (e) => { |
There was a problem hiding this comment.
This function still gets called by the button even though inputs are valid?
| else{ | ||
| setSpeedValid(true) | ||
| setSpeed(e.target.value) | ||
| console.log(speed) |
| } | ||
| const weatherChangeHandler = (e) => { | ||
| weatherChange(e.target.value); | ||
| console.log(weather) |
| setInitialCalc(true); | ||
| } | ||
| const speedChangeHandler = (e) => { | ||
| if(e.target.value > 90 || e.target.value < 0){ |
There was a problem hiding this comment.
We should be validating if the input is a number or not, I was able to type in "e" and "-" into the input box. This is because the number input can accept floating-point numbers (e)
| } | ||
| const batterChangeHandler = (e) => { | ||
| if(e.target.value > 100 || e.target.value<0) { |
There was a problem hiding this comment.
We should be validating if the input is a number or not, I was able to type in "e" and "-" into the input box. This is because the number input can accept floating-point numbers (e)
| @@ -1,8 +1,9 @@ | |||
| const BatteryInput = () => { | |||
| const BatteryInput = (props) => { | |||
| return ( | |||
| <div className="flex w-full flex-col items-center gap-2"> | |||
| <label>Battery Percentage (%):</label> | |||
There was a problem hiding this comment.
In App.tsx, you give the battery an intial value of 0, but when the app mounts for the first time, there is no initial value for the battery input.
Why is this? Look into controlled input fields work in React
| @@ -1,9 +1,10 @@ | |||
| const SpeedInput = () => { | |||
| const SpeedInput = (props) => { | |||
| return ( | |||
| <> | |||
| <div className="flex w-full flex-col items-center gap-2"> | |||
| <label>Speed (km/h):</label> | |||
| <input | |||
There was a problem hiding this comment.
In App.tsx, you give the battery an intial value of null, but the input field is of type "number". This can cause a type error in the future.
| const[speedValid,setSpeedValid]= useState(false); | ||
| const[batteryValid,setBatteryValid]= useState(false); |
There was a problem hiding this comment.
We're creating 2 state variables to check to see if the 2 inputs is valid. What if we had a form that had 50+ inputs, are we expecting to create 50+ corresponding valid states? There should be a better way to check if the inputs are valid without creating extra state.
| const [calc, setCalc] = useState(0) | ||
| const [initialCalc, setInitialCalc] =useState(false); |
There was a problem hiding this comment.
Do we need to create extra state to store the calculated range? We should be able to derive the range from the 3 state variables speed, batteryPercent, weather.
Remember, when you update state in React, it will cause your whole component to re-render (unless you use useMemo or useCallback...will get to that in future lesson). Think about using this to your advantage and you may not need extra state to store the range.
Telemetry should be in there now