Learning JavaScript - calculator

Question

I'm learning JavaScript and as a start I'm trying to create a calculator.

window.onload = function() {

  var numbers = document.getElementsByTagName('span');
  var result = document.querySelectorAll('.result')[0];
  var clear = document.getElementsByClassName('clear')[0];

  for (var i = 0; i < numbers.length; i += 1) {
    if (numbers[i].innerHTML === '=') {
      numbers[i].addEventListener("click", calculate(i));
    } else {
      numbers[i].addEventListener("click", addValue(i));
    }
  }

  clear.onclick = function() {
    result.innerHTML = '';
  };


  function addValue(i) {
    return function() {
      if (numbers[i].innerHTML === '÷') {
        result.innerHTML += '/';
      } else if (numbers[i].innerHTML === 'x') {
        result.innerHTML += '*';
      } else {
        result.innerHTML += numbers[i].innerHTML;
      }
    };
  }

  function calculate(i) {
    return function() {
      result.innerHTML = eval(result.innerHTML);
    };
  }
};

	* {
	  margin: 0;
	  padding: 0;
	  box-sizing: border-box;
	  font: bold 15px Arial, sans-serif;
	}
	#calculator {
	  width: 312px;
	  height: auto;
	  margin: 100px auto;
	  padding: 10px 10px 10px 10px;
	  background: #0088cc;
	}
	.top span.clear {
	  float: left;
	}
	.top .result {
	  height: 40px;
	  width: 212px;
	  float: left;
	  background: rgba(0, 0, 0, 0.2);
	  border-radius: 3px;
	  font-size: 17px;
	  line-height: 40px;
	  color: white;
	  text-shadow: 1px 1px 2px rgba(0, 0, 0, 0.2);
	  text-align: right;
	  letter-spacing: 1px;
	}
	.keys,
	.top {
	  overflow: hidden;
	}
	.keys span,
	.top span.clear {
	  float: left;
	  position: relative;
	  top: 0;
	  cursor: pointer;
	  width: 66px;
	  height: 36px;
	  background: white;
	  border-radius: 3px;
	  margin: 0 7px 11px 0;
	  line-height: 36px;
	  text-align: center;
	  transition: all 0.2s ease;
	}
	span.equ {
	  background: #00FF19;
	}
	.top span.clear {
	  background: red;
	  color: white;
	  font-size: 17px;
	}
	.keys span:active {
	  top: 3px;
	}
	.top span.clear:active {
	  top: 3px;
	}
	

<div id="calculator">
  <div class="top">
    <span class="clear">C</span>
    <div class="result"></div>
  </div>
  <div class="keys">
    <span>7</span>
    <span>8</span>
    <span>9</span>
    <span>+</span>

    <span>4</span>
    <span>5</span>
    <span>6</span>
    <span>-</span>

    <span>1</span>
    <span>2</span>
    <span>3</span>
    <span>÷</span>

    <span>0</span>
    <span>.</span>
    <span class="equ">=</span>
    <span>x</span>


  </div>
</div>

Answer 1

    * {
      margin: 0;
      padding: 0;
      box-sizing: border-box;
      font: bold 15px Arial, sans-serif;
    }

You should avoid the * if possible. The issue with * is that it affects all elements on the page, yours or otherwise. Consider using normalize.css to make default element styling consistent in a non-destructive manner.

.keys,
.top

For this app, this looks fine. Putting this on another page with existing markup, you can potentially clobber other styles (or get yours clobbered) because the class names are too generic. Also, very specific CSS (nested selectors) increase specificity. CSS written like this isn't easy to override. Consider using BEM naming scheme instead. It allows you to construct low-specificity selectors as well as make them meaningful.

transition: all 0.2s ease;

Assuming you can CSS3 or don't care about older browsers. The problem with all is that it applies to all. If you happen to change another property which also happens to be transitionable, this can catch you off guard. Consider specifying the specific property to transition.

window.onload = function() {

Nothing wrong here. Just wanted to point out that window.onload is probably the very last event to fire during the page load process. It also fires only after all images have loaded, which can take very long. Consider usign DOMContentLoaded event instead.

    clear.onclick = function() {
      result.innerHTML = '';
    };

One issue I see when doing handlers this way is that what if you want to add more handlers. Simply assigning a function to the event would only assign one at a time. In times where you want more handlers, this can be tricky. Use addEventListener instead.

var numbers = document.getElementsByTagName('span');
var result = document.querySelectorAll('.result')[0];
var clear = document.getElementsByClassName('clear')[0];

querySelectorAll has a single-element counterpart: querySelector. Use that for single-element returns. And while you're at it, you can forget the getElementsByTagName and getElementsByClassName, use querySelector and querySelectorAll for consistency. Performance losses are negligible in this case.

for (var i = 0; i < numbers.length; i += 1) {
  if (numbers[i].innerHTML === '=') {

= isn't a number. You should probably remove it from the numbers collection to avoid complexity in the code. Treat it like your clear.

Also, consider converting numbers to a proper array. This is possible using Array.prototype.slice.call(numbers). This way, you can easily use the forEach method to loop through the elements instead of constructing a fiddly for loop.

result.innerHTML = eval(result.innerHTML);

This can be tricky, not because eval is very powerful (that's a different issue), but because you're allowing execution of an arbitrary expression that may be interpretted very differently by JavaScript. One example is mentioned in the comments, where the numbers may be treated as octal. 011 + 2 isn't 13 but instead, 9 + 2 which is 11.

Since you are not separating the numbers explicitly anywhere for us to use parseFloat on each one to get the correct number representation, consider building a simple expression parser.