Arreglar código para que funcione y simplificar

Question

El código de esta página no funciona porque no consigo llamar a los botones desde Javascript e identificar que acción está haciendo cada uno de ellos. Los botones los llamo desde un addEventListener para obtener su estado, pero a la hora de iniciar el setInterval para cada botón la variable i siempre es 8, por tanto no puedo saber que botón es el que he pulsado.

quiero que haga lo mismo que el código de abajo pero llamando las funciones desde Javascript y simplificándolo al máximo.

let posImg = 0;
let posImg2 = 0;
let tiempo = 0;
let intervalId;

function mover(boton) {
  intervalId = setInterval(function() {
    mientrasPresione(boton);
  }, 100);
}

function parar() {
  clearInterval(intervalId);
}


function mientrasPresione(boton) {
  switch (boton) {
    case 1:
      posImg++;
      break;
    case 2:
      posImg--;
      break;
    case 3:
      posImg2++;
      break;
    case 4:
      posImg2--;
      break;
    default:
      document.getElementById('cont1').innerHTML = "no funciona";
      break;
  }
  tiempo++;
  document.getElementById('cont1').innerHTML = posImg2 + " " + posImg + " " + tiempo;

}

.joystick path:hover {
  fill: red;
}

.joystick path:active {
  fill: black;
}

<div id="cont1"></div>
<svg class="joystick" width="30mm" height="30mm" viewBox="0 0 30 30">
  <g fill=blue>
    <path d="m20 10v-5l-5-5-5 5v5z" onmousedown="mover(3)" onmouseup="parar()" onmouseout="parar()"/>
    <path d="m20 20v5l-5 5-5-5v-5z" onmousedown="mover(4)" onmouseup="parar()" onmouseout="parar()"/>
    <path d="m10 10h-5l-5 5 5 5h5z" onmousedown="mover(2)" onmouseup="parar()" onmouseout="parar()"/>
    <path d="m20 10h5l5 5-5 5h-5z" onmousedown="mover(1)" onmouseup="parar()" onmouseout="parar()"/>
  </g>
</svg>

Intentaba iniciar los botones de esta forma pero no puedo

window.onload = function () {
           
            const panel = document.querySelectorAll('.joystick path');
            for (var i = 0; i < 8; i++) {
               var cc= panel[i].addEventListener('mousedown', () => {
                    intervalId[i] = setInterval(function(){ mientrasPresione(i); }, 100);
                });
                panel[i].addEventListener('mouseup', () => {
                    clearInterval(intervalId[i]);
                });
            }
        };

Si declaras la variable `i` utilizando `let` (no `var`) vas a obtener el resultado esperado. Más info [aquí](https://es.stackoverflow.com/questions/56116/cuando-conviene-utilizar-var-let-y-const-en-ecma-script-6). Así mismo ¿por que `8`? Deberías iterar el arreglo `panel` para evitar controlar de mas. — Marcos, Sep 29 '20 at 11:55
Ten en cuenta que este código: `function () { mientrasPresione(i); }` no se va a ejecutar hasta que el intervalo "salte", y solo en ese momento se evaluará cuánto vale `i`. Otra manera de solucionar tu problema es utilizar este código: `(function (n) { return function () { mientrasPresione(n); }; })(i)`. De esta forma estarás creando 8 funciones diferentes, cada una con un valor de `n`. — jotaelesalinas, Sep 29 '20 at 12:57

score 2 · Accepted Answer · edited Sep 30 '20 at 02:31

Para quitar los eventos en línea, te sugiero usar atributos de datos, especificando la dirección en que apunta el botón, básicamente, cambiar onmousedown="mover(X)" onmouseup="parar()" onmouseout="parar()" por data-direction="X". Puedes acceder fácilmente al valor con elemento.dataset.direction.

Otra modificación es no usar window.onload = function() { ... }, prefiriendo el evento DOMContentLoaded para asignar eventos a los botones en cuanto sea posible y no esperar la carga de otros elementos (CSS, imágenes, etc.).

En lugar de crear un ciclo for, se puede usar forEach, para tener el botón y sus atributos disponibles en ese contexto, se toma el atributo direction para pasar a los eventos y se definen 4:

mousedown: Inicia el temporizador
mouseup: Cancela el temporizador
mouseout: Cancela el temporizador cuando el cursor sale del botón, porque mouseup se dispararía en otro contexto/elemento, no en el botón
click: Cancela el temporizador, porque se dispara solo mousedown, pero no mouseup

Nota: Los 4 eventos son necesarios de acuerdo a las pruebas que hice ejecutando el fragmento de código, pero deberías realizar pruebas más amplias para ver si el comportamiento es el adecuado.

let posImg = 0;
let posImg2 = 0;
let tiempo = 0;
// Crear arreglo para intervalos
let intervalId = [];

function mientrasPresione(boton) {
  switch (boton) {
    case 1:
      posImg++;
      break;
    case 2:
      posImg--;
      break;
    case 3:
      posImg2++;
      break;
    case 4:
      posImg2--;
      break;
    default:
      document.getElementById('cont1').innerHTML = "no funciona";
      break;
  }
  tiempo++;
  document.getElementById('cont1').innerHTML = posImg2 + " " + posImg + " " + tiempo;

}

// Ejecutar hasta que cargue el DOM
document.addEventListener('DOMContentLoaded', function() {
    const panel = document.querySelectorAll('.joystick path');
    // Recorrer por botón, no por índice
    panel.forEach(button => {
        // Obtener dirección desde atributo de datos, forzado a entero
        let direction = parseInt(button.dataset.direction);
        // Asignar eventos
        button.addEventListener('mousedown', (e) => {
            e.preventDefault();
            intervalId[direction] = setInterval(function() {
                mientrasPresione(direction);
            }, 100);
        });
        button.addEventListener('mouseup', (e) => {
            e.preventDefault();
            clearInterval(intervalId[direction]);
        });
        // Si se arrastra el mouse fuera, mouseup no se dispara
        button.addEventListener('mouseout', (e) => {
            e.preventDefault();
            clearInterval(intervalId[direction]);
        });
        // Este es necesario para evitar que el intervalo siga ejecutándose
        // clic sencillo dispara mousedown, pero no mouseup
        button.addEventListener('click', (e) => {
            e.preventDefault();
               e.preventDefault();
             mientrasPresione(direction);
        });
    });
});

.joystick {
  user-select: none;
}

.joystick path:hover {
  fill: red;
}

.joystick path:active {
  fill: black;
}

<div id="cont1"></div>
<svg class="joystick" width="30mm" height="30mm" viewBox="0 0 30 30">
  <g fill=blue>
    <path d="m20 10v-5l-5-5-5 5v5z" data-direction="3" />
    <path d="m20 20v5l-5 5-5-5v-5z" data-direction="4" />
    <path d="m10 10h-5l-5 5 5 5h5z" data-direction="2" />
    <path d="m20 10h5l5 5-5 5h-5z" data-direction="1" />
  </g>
</svg>

Actualización:

Para evitar que el texto se seleccione al hacer clic repetidas veces, se agrega el evento como parámetro en las funciones y se agrega e.preventDefault();.

Adicionalmente, se puede desactivar la selección de texto en contenedores específicos por medio de la propiedad user-select de CSS.

esta bien pero como evitar que al dar muchos clicks se seleccione el texto de alrrededor del boton? — perico palote, Sep 29 '20 at 22:59
@pericopalote, ¿cuál texto? Esa parte no la mencionaste al preguntar. Podría enviarse el evento como parámetro a cada funcion y usar `.preventDefault()`, pero, sin ver de qué se trata, va a ser difícil responder. — Triby, Sep 29 '20 at 23:02
por ejemplo al pulsar el botón superior del joystick varias veces seguidas se queda seleccionado los números del contador de los botones, osea el texto del — perico palote, Sep 29 '20 at 23:17
Esta pregunta ya es una tontería, por que no me hace falta saberlo, pero si quieres respóndela. ¿existe una forma de que al pulsar el boton varias veces no se seleccionen los textos de alrededor, y que los textos de alrededor si sean seleccionables por si quiero hacer copy paste? — perico palote, Sep 29 '20 at 23:33
@pericopalote, solo quita la propiedad CSS y con eso debe ser suficiente. — Triby, Sep 29 '20 at 23:39

Pablo Lozano · Answer 2 · 2020-09-30T07:17:26.660

Se me ocurre que podrías añadir la información sobre el comportamiento de cada botón o flecha en el propio elemento, como dataset (atributos data-XXX):

let tiempo = 0;
// Objeto para controlar la posición actual y representarla en pantalla
// como coordenadas
const coords = {
  x: 0,
  y: 0,
  updateView: function() {
    document.getElementById('cont1').innerHTML = this.x + ' ' + this.y + ' ' + tiempo;
  }
};

let intervalId;

// inicializa el movimiento, usando los parámetros como velocidad
function mover(x, y) {
  coords.x += x;
  coords.y += y;
  coords.updateView();
  tiempo++;
}

// termina el movimiento
function parar() {
  clearInterval(intervalId);
}

const controls = document.querySelectorAll('.joystick path');
//para cada control
controls.forEach(function(control) {
  //leemos sus datos asociados
  const x = +control.dataset.x; //equivalente a parseInt
  const y = +control.dataset.y;
  // al pulsar se inicia el movimiento
  control.addEventListener('mousedown', () => {
    intervalId = setInterval(function() {
      mover(x, y);
    }, 100);
  });
  control.addEventListener('mouseup', () => {
    parar();
  });
  control.addEventListener('mouseout', () => {
    parar();
  });
  control.addEventListener('click', () => {
    mover(x, y);
  });
});

.joystick {
  user-select: none;
}

.joystick path:hover {
  fill: red;
}

.joystick path:active {
  fill: black;
}

<svg class="joystick" width="30mm" height="30mm" viewBox="0 0 30 30">
  <g fill=blue>
    <path d="m20 10v-5l-5-5-5 5v5z" data-y="-1" data-x="0"/>
    <path d="m20 20v5l-5 5-5-5v-5z" data-y="1" data-x="0"/>
    <path d="m10 10h-5l-5 5 5 5h5z" data-y="0" data-x="-1"/>
    <path d="m20 10h5l5 5-5 5h-5z"  data-y="0" data-x="1"/>
  </g>
</svg>

<div id="cont1"></div>

El click simple no siempre responde, ¿alguna forma de arreglarlo? — perico palote, Sep 30 '20 at 00:41

Arreglar código para que funcione y simplificar

2 Answers2

Actualización:

Linked