JS Calculator {Feedback}

Hi, here is my project: JavaScript Calculator
I appreciate any kind of feedback.
Edit: "console.log"s are just easter eggs. They don’t mean anything :slight_smile:
My code:

Main component:

import './App.css';
import React from 'react';
import {Buttons} from './Components/numbers'
import {evaluate, round} from 'mathjs'

const myRegex = /^[x+/-]/;
const myRegexOp = /[x+/-]\s$/;
let numArray = [];

class App extends React.Component {
  constructor(props){
    super(props);
    this.state = {
      formula: 'A JavaScript Calculator',
      display: ''

    }
    this.handleNumbers = this.handleNumbers.bind(this);
    this.handleClear = this.handleClear.bind(this);
    this.handleOperator = this.handleOperator.bind(this);
    this.clearFormula = this.clearFormula.bind(this);
    this.handleDecimals = this.handleDecimals.bind(this);
    this.finishHim = this.finishHim.bind(this);
    
  }
 
  clearFormula(){
    this.setState({
      formula: ''
    })
      }
  handleNumbers(x){
    if (this.state.display.length > 22){

    } else {
     if (numArray.length == 1){
       this.setState({
         formula: x.target.value,
         display: x.target.value
       })
       numArray = [];
     } else {
      const value = x.target.value;
      if (myRegex.test(this.state.display)){
      this.setState({
        display: value,
        formula: this.state.formula.concat(value)
    })
  } else {
    this.setState({
      display: this.state.display.concat(value).replace(/^0{2}/g, '0').replace(/^0[1-9]/, value),
      formula: this.state.formula.concat(value).replace(/[a-wyz]*/gi, "").replace(/^0{2}/, "0").replace(/^0[1-9]/, value).replace(/\s0{2}/, " " + "0").replace(/\s0[1-9]/g, " " + value)
  })

  }}
    }}
    handleOperator(y) {
      if(this.state.formula === 'A JavaScript Calculator'){
        console.log("Napıyon mq");
      } else { 
        
      if(this.state.display.length > 0) {
        if(this.state.display !== "+" & this.state.display !== "-" & this.state.display !== "x" & this.state.display !== "/") {
          numArray.push(this.state.display);
          numArray.push(y.target.value);
          
        }
      } if (y.target.value == "+" & numArray[numArray.length - 2] == "x" | y.target.value == "/" & numArray[numArray.length - 2] == "x" | y.target.value == "x" & numArray[numArray.length - 2] == "x" | numArray[numArray.length - 1] == "-" & numArray[numArray.length - 2] == "x"){
        numArray.splice(numArray.length - 2, 1,);
      }
      if (this.state.display == "x" & y.target.value == "-" & numArray[numArray.length - 1] !== "-" & numArray[numArray.length - 2] !== "x") {
        numArray.push(y.target.value);
        
      } 
      
      if (this.state.display == "+" | this.state.display == "-" | this.state.display == "x" | this.state.display == "/" & numArray.length !== 1){
        
        numArray.splice(numArray.length - 1, 1, y.target.value);
      
      } else if(numArray.length == 1){
        numArray.push(y.target.value);
      }
      
      const operatorValue = y.target.value;

      this.setState({
        display: operatorValue,
      })
      if (myRegexOp.test(this.state.formula)){
        if (/[x]\s$/.test(this.state.formula) & operatorValue == "-"){
          this.setState({
            formula:this.state.formula.concat("-" + " ")
          })
          
        } else {
          
        this.setState({
          formula: this.state.formula.replace(/x\s-\s$/, operatorValue + " ").replace(/[x+-/]\s$/, operatorValue + " ").replace(/[.]0+\s/g, " ").replace(/[0-9]*[.]\s/g, " ")
        })
        
      }} else {
        this.setState({
          formula: this.state.formula.concat(" " + operatorValue + " ").replace(/[.]\s/, " ").replace(/[.]0+\s/g, " ")
        })
      }
      console.log(numArray);
    }
    }
    handleDecimals(){
      if (numArray.length == 1){
        if (!this.state.formula.includes(".")){
        this.setState({
          formula: this.state.formula.concat("."),
          display: this.state.formula.concat(".")
        })
        numArray = [];
      } 
      } else {
      if (!this.state.display.includes(".")){
        this.setState({
          display: this.state.display.concat(".").replace(/^[.]/, "0."),
          formula: this.state.formula.concat(".").replace(/[a-wyz]*/gi, "").replace(/\s[.]/g, " " + "0.").replace(/^[.]/, "0.")
        })
        if (myRegex.test(this.state.display)){
          this.setState({
            display: "0.",
            
          })
        }
      
        
      }
    }
    } 
  
  handleClear(){
    this.setState({
      display: '0',
      formula: ''
    })
    numArray = [];
  }
  finishHim(){
    
      
    if (this.state.display.length == 0){
      this.setState({
        formula: this.state.formula
      })
    } else {
    if (!myRegex.test(this.state.display)){
      numArray.push(this.state.display)
    
    }
    if (numArray[numArray.length - 1] == "-" & numArray[numArray.length - 2] == "x"){
      numArray.splice(numArray.length - 2,2,)
      console.log("Nice Try");
    }
    let result = round(evaluate(numArray.join("").replace(/[x+/-]$/, ("")).replace(/x/g, ("*"))), 14)
    
    this.setState({
      formula: String(result),
      display: ''
    })
    
    numArray.splice(0,numArray.length,String(result))
  }

  }
  
  render() {
    return(
      <div className="container">
        <div id="calculator">
        <div id="formula">{this.state.formula}</div>
      <div id="display">{this.state.display}</div>
      <Buttons
      numbers={this.handleNumbers}
      operators={this.handleOperator}
      clear={this.handleClear}
      decimals={this.handleDecimals}
      finish={this.finishHim}
      ></Buttons>
      <footer>Coded by Ozan</footer>
      </div>
      
      </div>
    )
  }
}



export {App};


Child component:

import React from 'react';

class Buttons extends React.Component {
    constructor(props){
        super(props);
        this.state = {
            
        }
        
    }


    render(){
        return(
            <div id="buttons">
            <button class="number" value="=" onClick={this.props.finish} id="equals">=</button>
            <div id="numbers">    
            <button class="number" value = "1" onClick={this.props.numbers} id="one">1</button>
            <button class="number" value = "2" onClick={this.props.numbers} id="two">2</button>
            <button class="number" value = "3" onClick={this.props.numbers} id="three">3</button>
            <button class="number" value = "4" onClick={this.props.numbers} id="four">4</button>
            <button class="number" value = "5" onClick={this.props.numbers} id="five">5</button>
            <button class="number" value = "6" onClick={this.props.numbers} id="six">6</button>
            <button class="number" value = "7" onClick={this.props.numbers} id="seven">7</button>
            <button class="number" value = "8" onClick={this.props.numbers} id="eight">8</button>
            <button class="number" value = "9" onClick={this.props.numbers} id="nine">9</button>
            <button class="number" value = "0" onClick={this.props.numbers} id="zero">0</button>
            <button class="number" id="decimal" value="." onClick={this.props.decimals} >.</button>
            </div>
            <button id="add" value= "+"   onClick={this.props.operators}>+</button>
            <button id="subtract" value= "-" onClick={this.props.operators}>-</button>
            <button id="multiply" value= "x" onClick={this.props.operators}>*</button>
            <button id="divide"  value= "/" onClick={this.props.operators}>/</button>
            <button id="clear" onClick={this.props.clear}>AC</button>
            </div>
        )
    }
}

export {Buttons};

Hey, just a few suggestions.

  • On my browser (Firefox on Linux) I can’t see the keyboard focus indicator at all and thus I don’t know where the current focus is set. I think you should make the focus indicator more prominent. If you think that’s ugly then you can only show it to keyboard users only using the :focus-visible pseudo-class (:focus-visible and backwards compatibility | TPG – The Accessibility Experts).
  • When I increase the text size to 200% the numbers in the displays get cut-off. In general you should use em (or rem) units when setting width/height on elements that contain text so that they grow as the text size increases. If you don’t know how to increase the text size manually, search for ‘text only zoom’.

I’m hesitant to list this last one because I don’t think it is a requirement and my solution might violate the spirit of the challenge. I’ll just throw it out there so you are aware (if you weren’t already).

  • Your calculator suffers from the dreaded JS floating point imprecision. Try the following: 0.1 + 0.2. I don’t know if this is acceptable for the challenge but as I perfectionist I wouldn’t be happy with it :slight_smile: My solution would be to use a math library like mathjs.
1 Like

I like the feedback and made changes, if you want to check it out I appreciate it.
Here: JavaScript Calculator

The keyboard focus issue is definitely fixed and 0.1 + 0.2 gives the correct answer now.

You didn’t fix the display issue for larger text sizes though. At 200% I can’t really see what I’m entering into the calculator or the result it is giving me.

I set the font-size to 1em. Couldn’t be exactly sure how to solve that.

So it’s not the font-size that needs to be fixed. On the divs you are using for the display you have them both set to height: 40px;. This is problem. As long as the text height is less than that it will fit in the div, but as soon as the text is taller than that it will start getting cut-off at the bottom of the div. On my browser, at a 200% increase I see only the top half of the text.

If you don’t know how to manually increase the text size then search for “zoom text only” along with the browser you are using. The important point here is that the viewer ultimately has control over the text size they need to use to see your content and your page should be able to handle increases in text size gracefully. In general, if you need to set the height/width on an element that contains text then you should use em (or rem) units so that the element can grow as the text size increases.

Noted. Behalf of responsiveness, it is a good knowledge.

I will set height and width of the elements that has readable text according to from now on.