Soccer field map creator

Question

I am fairly new to OOP and am working on a little website project to practice. I wanted to post after getting the initial setup done in case I am making any major errors or not following best practices.

Feedback Requested Looking for feedback mostly on the OOP aspect. Since I am new I don't have a full understanding I'll take any suggestions of what I need to learn about. You can give me terms to search for, article a bonus. Of course I'm more than happy to see any code reviews on this, just thought it would be more beneficial to focus on getting a better understanding.

Code Purpose Create a soccer field on the map by clicking the "Add Field" button and then clicking on the map. This is not a final version, so will be working on UI and more features such as size & rotation of the fields, and interacting with ones already placed (moving, resizing, removing, etc).

CodePen Live Example: https://codepen.io/BadMoodTaylor/pen/KyQoea

// setup Map object
var Map = {};
Map.clickFunction = '';
Map.left = $('.map-container').position().left;
Map.top = $('.map-container').position().top;
Map.fields = [];
Map.id = $('.map-container');


class Field{
  constructor(num, x, y){
    // assigning each field a unique class name
    this.id = 'field' + num;
    // copying the HTML from the template
    this.html = '<div class="svg-field ' + this.id + '">' + $('#template').html() + '</div>';
    // Adding the HTML to the map
    Map.id.append(this.html);
    // Storing Height and Width
    this.height =   $('.' + this.id).height()
    this.width =   $('.' + this.id).width();
    // Setting location based on where user clicked.  Setting middle of field to click point
    this.x = x - this.width / 2;
    this.y = y - this.height / 2;
    $('.' + this.id).css('left', this.x);
    $('.' + this.id).css('top', this.y);
  }

// not used yet, will be to reposition field
  position(x, y){
    this.x = x;
    this.y = y;

  }
}



  // Add Field Button Clicked
  $('.add-field').click(function(){
    Map.clickFunction = 'addField';
    console.log("Add Field button pressed and Map.clickFunction set to: " + Map.clickFunction)
  })

  // Click on Map
  $('.map-container').click(function(e){
    // Finding desired action based on Map.clickFunction
    switch (Map.clickFunction) {
      case 'addField':
        // Reset parameter
        Map.clickFunction = '';

        // Create new field
        let newField = new Field(Map.fields.length, e.pageX - Map.left, e.pageY - Map.top);
        Map.fields.push(newField);

        break;
      default:
        console.log('default')

    }
  })

  body{
    display: flex;
    flex-flow: column;
    justify-content: center;
    align-items: center;
  }

  .lines{
    fill:none;
    stroke-width: 1;
    stroke: white;
    animation: 1.4s 0.1s both drawLines;
    stroke-dasharray: 100 0;
  stroke-dashoffset: 0;
  }
  @keyframes drawLines {
    from{
          stroke-dasharray: 1 100;
      stroke-dashoffset: 10;
    }
  }
  .map-container{
    position: relative;
    width: 1000px;
    height: 700px;
    background: #3d9156;
  }
  .svg-field{
    position: absolute;
    top: 60%;
    left: 6%;
    z-index: 5;
    width: 70px;
    height: 120px;
  }
  .svg-field svg{
    position: absolute;
    top:0;
    left:0;
    height: 120px;
    width: 70px;
  }
  .hidden{
    display: none;
  }
  .spot{
    fill:white;
  }
  .penalty-arc{
    stroke-dasharray: 18 50;

  }
  .penalty-arc.top-half{
        stroke-dashoffset: -6.5;
  }
  .penalty-arc.bottom-half{
    stroke-dashoffset: -38;
  }


  .div-button:hover {
    transform: translateY(5px);
    box-shadow: 0 0 0 #e67409;
    text-shadow: 2px 2px 0 #42964c;
    background-color: #69c160;
}

.div-button {
    background-color: #69c160;
    color: white;
    border-radius: 10px;
    padding: 0.5em;
    font-size: 1.5rem;
    cursor: pointer;
    box-shadow: 0 5px 0 #42964c;
    transition-duration: 0.2s;
    text-shadow: 2px 2px 0 #42964c;
    animation: 0.6s 5.8s both button1;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>
<div class="map-container">
  <div id="template" class="svg-field">
      <svg version="1.1"
           baseProfile="full"
           width="100%" height="100%"
           viewBox="0 0 70 120"
           xmlns="http://www.w3.org/2000/svg">

          <!--Perimeter (Touch Lines and Goal Lines) -->
          <rect class="lines" x="0" y="0" width="100%" height="100%"/>
          <!--Center Circle -->
          <circle class="lines" cx="50%" cy="50%" r="10" />
          <!--Halfway line -->
          <line class="lines" x1="0" y1="50%" x2="100%" y2="50%" />
          <!--Penalty Spots -->
          <circle class="spot" cx="50%" cy="12" r="0.5" />
          <circle class="spot" cx="50%" cy="108" r="0.5" />
          <!--Goal Area -->
          <path class="lines" d="M 35 120 m -14 0 l 0 -6 l 28 0 l 0 14" />
          <path class="lines" d="M 35 0 m -14 0 l 0 6 l 28 0 l 0 -14" />
          <!--Penalty Areay -->
          <path class="lines" d="M 35 120 m -22 0 l 0 -18 l 44 0 l 0 18" />
          <path class="lines" d="M 35 0 m -22 0 l 0 18 l 44 0 l 0 -18" />
          <!--Penalty Arc -->
          <circle class="lines penalty-arc top-half" cx="50%" cy="12" r="10" />
          <circle class="lines penalty-arc bottom-half" cx="50%" cy="108" r="10" />
        </svg>
  </div>
</div>




<div class="div-button add-field">
  Add Field
</div>

Answer 1

That animations looks pretty cool. I'm no expert in OOP, but there is one thing I want to mention. This

this.html = '<div class="svg-field ' + this.id + '">' + $('#template').html() + '</div>';

is a bad idea. You might not always be sure what's passed to the function, and if it gets called like this

new Field('"><script>alert("HAX")</script>',100,100)

someone can inject any code, which is a major security risk. Instead you can do it like this

this.html = document.createElement('div');
this.html.classList.add("svg-field");
this.html.classList.add(this.id);
let templateClone = document.getElementById('template').cloneNode(true);
this.html.appendChild(templateClone);

I didn't use jQuery, but you could, since you already have it included. You also need a few tweaks, since the position changed a bit.

Answer 2

OOP feedback

The class only has two methods, and one isn't used (you even said so yourself: // not used yet, will be to reposition field). That position method could be called by the constructor, and other parts of the constructor could be moved out into other methods - e.g. one to create the element, set the styles, etc.

General feedback

If there is only one element that will contain the map, then perhaps using the id attribute instead of a class name would be more appropriate.

---

DOM queries aren't cheap:

”...DOM access is actually pretty costly - I think of it like if I have a bridge - like two pieces of land with a toll bridge, and the JavaScript engine is on one side, and the DOM is on the other, and every time I want to access the DOM from the JavaScript engine, I have to pay that toll”
    - John Hrvatin, Microsoft, MIX09, in this talk Building High Performance Web Applications and Sites at 29:38, also cited in the O'Reilly Javascript book by Nicholas C Zakas Pg 36, as well as mentioned in this post

Bearing in mind he said that ~10 years ago and browsers have come a long way, it is still wise to consider. It is best to store those references in a variable (e.g. const mapContainer = $('.map-container')) and utilize those whenever needed later.

---

const can be used for any variable that doesn't get re-assigned - e.g. newField. That way there won't be any accidental re-assignment.

---

The switch statement seems excessive for two cases (including the default) - it could easily be replaced by a simple if/else chain