Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

React: createContext(), hooks: useState(), useEffect()

Lukas 130

@lukKotecki

Desktop design screenshot for the Calculator app coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
3intermediate
View challenge

Design comparison


SolutionDesign

Solution retrospective


Some similar features are handled differently because I wanted to practice some skills. The most challenging part was to provide order of mathematical operations, for ex: 2+2*2=6 instead of 8. I handled this with an array of input because I also wanted to implement undo operation with 'DEL' button. It's clunky and heavy code but it works.

Community feedback

@jebbaited

Posted

If you don't mind I'll leave some comments to your code.

1: Use Prettier to format code. It is much easier to read when formatted. Even with default settings.

2: Use error-first approach if possible when writing conditions. Your code:

function removeLastInput(){
  if(input.length){
    setInput(input.filter((el,i)=> input.length-1 !==i && el ))
  }
}

My code:

  function removeLastInput() {
    if (!input.length) return

    setInput(input.filter((el, i) => input.length - 1 !== i && el))
  }

  function replaceLastInput(value) {
    if (!input.length) return

    setInput(input.map((el, i) => (input.length - 1 !== i ? el : value)))
  }

3: In switch/case construction you can use return instead of break. Less rows in general.

    if (e.target.value) {
      switch (value) {
        case 'RESET':
          return setInput([])

        case 'DEL':
          return removeLastInput()

        case '=':
          return calculateResult(concatNumbers())

        default:
          setCharToInput(value)
      }
    }

4: Variable creation. You could create an array in another way. Your code:

const firstArray = [...concatedNumbers]
const secondArray = []
.
.
.
firstArray.forEach((el) => el && secondArray.push(el))

My code:

const firstArray = [...concatedNumbers]
.
.
.
const secondArray = firstArray.map((el) => el)

Array.map() returns a new array and you don't need to push to another which you created above.

5: Some conditions difficult to understand and maintain. Like this one:

 if (
        previousInput === value ||
        !previousInput ||
        previousInput === '.' ||
        isNaN(input[0] && isNaN(input[1])) ||
        (isNaN(previousInput) && isNaN(input[input.length - 2]) && input.length >= 2) ||
        (input.length === 1 && previousInput === '-')
      )

Should be a good solution to create couple variables with good naming and then put them into this condition.

    const notNaN = isNaN(input[0] && isNaN(input[1]))
    const someName = isNaN(previousInput) && isNaN(input[input.length - 2]) && input.length >= 2

if (
        previousInput === value ||
        !previousInput ||
        previousInput === '.' ||
        notNaN ||
        someName ||
        (input.length === 1 && previousInput === '-')
      )

6: Try to not repeat similar components:

          <Button>7</Button>
          <Button>8</Button>
          <Button>9</Button>
          <Button className="del">DEL</Button>
          <Button>4</Button>
          <Button>5</Button>

Create an array and then go throught it.

  const Buttons = [
    {
      name: '7',
    },
    {
      name: '8',
    },
    {
      name: '9',
    },
    {
      name: 'DEL',
      class: 'del',
    },
    {
      name: '4',
    },
    {
      name: '5',
    },
    {
      name: '6',
    },
    {
      name: '+',
    },
    {
      name: '1',
    },
    {
      name: '2',
    },
    {
      name: '3',
    },
    {
      name: '-',
    },
    {
      name: '.',
    },
    {
      name: '0',
    },
    {
      name: '/',
    },
    {
      name: 'x',
    },
    {
      name: 'RESET',
      class: 'reset',
    },
    {
      name: '=',
      class: 'equal',
    },
  ]

        <Keyboard onClick={handleClick}>
          {Buttons.map((button) => (
            <Button className={button.class ? button.class : ''}>{button.name}</Button>
          ))}
        </Keyboard>

Also you don't need <Keyboard /> component. Just handle clicks on every button.

 <div className="kayboard">
          {Buttons.map((button, index) => (
            <Button className={button.class ? button.class : ''} onClick={handleClick} key={index}>
              {button.name}
            </Button>
          ))}
  </div>

7: Let's change <Button/> component a little bit.

export default function Button({ name, className, onClick }) {
  return (
    <button value={name} className={`button ${className}`} onClick={onClick}>
      {name}
    </button>
  )
}

Now you can use without children props. Cause why do you need it if you pass only button name.

  <Button
              className={button.class ? button.class : ''}
              onClick={handleClick}
              name={button.name}
              key={index}
    />

8: ThreeStateToggle. Here we'll also avoid using same <input /> couple times. Create <Input /> component:

const Input = forwardRef(function Input({ number, id, handleClick }, ref) {
  return (
    <>
      <span>{number}</span>
      <input
        ref={ref}
        onClick={handleClick}
        className="toggle"
        type="radio"
        name="toggle"
        id={id}
      />
    </>
  )
})

Then use it:

  const themeSwitchInputs = [
    { number: 1, id: 'dark', ref: dark },
    { number: 2, id: 'light', ref: light },
    { number: 3, id: 'violet', ref: violet },
  ]
.
.
.
 <div className="tri-state-toggle">
        {themeSwitchInputs.map((input) => (
         <Input
            number={input.number}
            id={input.id}
            handleClick={handleClick}
            ref={input.ref}
            key={input.number}
          />
        ))}
</div>

About forwardRef read DOCS

Marked as helpful

1
Lukas 130

@lukKotecki

Posted

Than You very much for review. It's very helpful and enlightening.

  1. done

  2. fixed

  3. fixed

  4. leaved as it was because I'm not sure how to delete null indexes

  5. fixed - even me when first written was scared of these conditions

  6. fixed - I like this one

  7. fixed

  8. fixed -still not sure how it works but copy & paste helps

Once again Thank You!

0

@jebbaited

Posted

@lukKotecki 4.

const secondArray = firstArray.filter((el) => !!el)
or 
const secondArray = firstArray.filter((el) => {
if (el !== null) return el
})
or
const secondArray = firstArray.filter((el) => el && el)

You are welcome!

Marked as helpful

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord