Yup. We all know the feeling.
It seems to work well, hastily looking at the code…
{buttons.map((btn, index) => {
return (
<Button key={index} btn={btn} input={input} setInput={setInput} currentNumber={currentNumber} setCurrentNumber={setCurrentNumber} operator={operator} setOperator={setOperator} equalPressed={equalPressed} setEqualPressed={setEqualPressed} />
)
It is generally considered bad practice to use the index as the key. something to be avoided. It’s better to use some unique value from your data. How about the button id? And most people would probably use an implicit return here, but that gets into stylistic choices.
<div className="keyboard">
<!-- ... -->
</div>
For organization, I would have put this section in it’s own component, mirroring the Display component. They should be hierarchically on the same level - but that’s getting nitpicky.
const handleClick = (value, type) => {
switch (type) {
case 'reset':
resetState();
break;
case 'negative':
reverseNumber();
break;
case 'zero': // do not allow number to start with zero
if (currentNumber === '0') {
break;
}
// eslint-disable-next-line
case 'number':
appendNumber(value);
break;
case 'operator':
appendOperator(value);
break;
case 'decimal':
addDecimal();
break;
case 'equals':
calculateSum();
break;
default:
return currentNumber;
}
}
There are a couple of oddities here. First of all, why are we returning anything from this? And if we return something sometimes, I think we would want to return something always, even if it’s just undefined.
And did you intend the fall-through after “zero”? For readability, if that’s what you want, I might do something like:
// ...
case 'zero':
case 'number':
if (currentNumber !== '0') {
appendNumber(value);
}
break;
// ...
To me that would be clearer - if that’s what you’re after.
setInput(`${newInput} <span class="operator"> ${op} </span> `);
I’m not sure how I feel about setting JSX in your state. I would rather set values and have the JSX constructed elsewhere. I don’t know that it is per se “wrong”, it just looks weird to me.
onClick={() => handleClick(btn.label, btn.class)}
It seems like that info could be stored on the button and gotten off the event in the handler. Then you wouldn’t have to wrap it in an anonymous function - that would be cleaner and more performant.
Oh well, it looks good. Those are just some hasty observations. Have fun on the next project.