Вопрос по программе онлайн пианино - не работает клавиша при клике
Всем привет! Опять проблемы с пианино :( Пишу приложение онлайн-пианино, вроде все сделала, кроме одного требования - при зажатии клавиши мыши на клавише, перемещаешь курсор на другие клавиши - играют другие ноты. Проблема с этим такая - перебор по клавишам работает, но не работает звук на той клавише, которую первой зажимаю. Если у кого-то есть минутка, можете глянь код? Понимаю, что где-то что-то лишнее написала, но не могу найти, что именно.
Вот js:
window.addEventListener("keydown", function (event) {
playSound(event.keyCode);
});
const piano = document.querySelector('.piano');
const pianoKeys = document.querySelectorAll('.piano-key');
const playSound = function (keyNumber) {
const audio = document.querySelector(`audio[data-key="${keyNumber}"]`);
const key = document.querySelector(`.piano-key[data-key="${keyNumber}"]`);
if (!audio) {
return;
}
audio.currentTime = 0;
audio.play();
key.classList.add('piano-key-active');
};
let removeTransition = function (event) {
if (event.propertyName !== 'transform') {
return;
}
this.classList.remove('piano-key-active');
};
pianoKeys.forEach(key => key.addEventListener('transitionend', removeTransition));
const clickPlaySound = function (event) {
const keyNumber = this.getAttribute("data-key");
playSound(keyNumber);
};
const buttons = document.querySelectorAll('.btn');
for (let button of buttons) {
button.addEventListener('click', function () {
if (this.classList.contains('btn-active')) return;
buttons.forEach(i => i.classList.remove('btn-active'));
this.classList.toggle('btn-active');
piano.classList.toggle('change-style');
});
}
const startCorrespondOver = (event) => {
event.target.classList.add('piano-key-active');
pianoKeys.forEach((key) => {
key.addEventListener('mouseover', clickPlaySound);
key.addEventListener('mouseout', removeTransition);
});
};
const stopCorrecpondOver = () => {
pianoKeys.forEach((key) => {
key.classList.remove('piano-key-active');
key.removeEventListener('mouseover', clickPlaySound);
key.removeEventListener('mouseout', removeTransition);
});
};
for (let i = 0; i < pianoKeys.length; i++) {
pianoKeys[i].addEventListener('mousedown', startCorrespondOver);
pianoKeys[i].addEventListener('mouseup', stopCorrecpondOver);
}
Ответы (2 шт):
Ну у вас ведь в startCorrespondOver нигде не вызывается clickPlaySound для этой клавиши, ну которую зажали.
Вот так должно заработать
...
const clickPlaySound = function (event) {
const keyNumber = event.toElement.getAttribute("data-key");
playSound(keyNumber);
};
const startCorrespondOver = (event) => {
clickPlaySound(event);
event.target.classList.add('piano-key-active');
pianoKeys.forEach((key) => {
key.addEventListener('mouseover', clickPlaySound);
key.addEventListener('mouseout', removeTransition);
});
};
...
Ксения, привет! Дам ответ и дам немного конструктивной критики.
Где неправильно:
- при нажатии мышкой на клавишу произойдёт событие
'startCorrespondOver'
pianoKeys[i].addEventListener('mousedown', startCorrespondOver);
- только после нажатия мышкой в каждую из клавиш добавится событие 'mouseover'
key.addEventListener('mouseover', clickPlaySound);
Но в нажатую клавишу мышка уже не зайдёт, она уже в ней! Она только выйдет из неё.
Решение "в лоб":
Вызвать clickPlaySound сразу по 'mousedown' по клавише:
pianoKeys[i].addEventListener('mousedown', ()=> {
clickPlaySound(event);
startCorrespondOver(event);
);
Тут в качестве колбэка используется анонимная стрелочная функция ()=>{doSomething()}
Теперь советы. Расположу их в порядке усложнения.
- Совет на вашем уровне умения кодить: научитесь логгировать и дебажить.
Если бы после каждого вызова события вызывали
console.log, например для'mouseover'
console.log("На клавишу:"+key+"наехала мышка, вызываем функцию PlaySound")
вы бы увидели, что просто не происходит нужного события.
- Всё дело в том, что вы, судя по всему, разрабатываете прототип и не задумывались об архитектуре и о чистом коде. Изучайте постепенно (но начинайте уже сейчас) чистый код
Вот пример. Проблема в том, когда вы вызываете эту функцию.
buttons.forEach(i => i.classList.remove('btn-active'));
Поясню. Существует концепция S.O.L.I.D. Следуя последней D., функция, вызванная из модуля нижнего уровня иерархии (один key или button) не должна управлять иерархией более высокого уровня -- всей остальной клавиатурой (keys или buttons).
Эта функция должна быть вызвана по 'mouseup' (используйте анонимную стрелочную функцию, как в решении выше), и не внутри клавиши, а внутри всего пианино (поясню ниже)
- Архитектурный подход. Сейчас ваши модули и элементы это просто однородная россыпь предметов. Будто из сумки высыпали все предметы (в том числе предметы из косметички, все монеты из кошелька и т.д.) на стол и они там хаотично валяются. При этом каждый из предметов может неожиданно повлиять на все другие предметы. Например, лак для ногтей протечь и запачкать паспорт. Пока приложение малюсенькое, за этим легко следить. Но кнопка, которая посылает сигнал управления всем остальным кнопкам -- это неправильно. Подписывать всех слушателей на каждую из кнопок это не правильно. Каждая работа должна выполняться на своём уровне иерархии, не выше и не ниже.
Вам нужно создать один единственный объект пианино и на <div id='piano> навешивать слушателей 'mousedown' и 'mouseup'. Далее из event по event.taget брать id клавиши и передавать этот id отдельно в функцию, управляющую состоянием пианино (какие клавиши в каком состоянии -- аналог того, что вы делаете через атрибуты класса).
'click' вам не нужен нигде, достаточно 'mouseup'
'mouseover' и 'mouseout' действительно нужны на каждой клавише.
Эти три слушателя так же должны изменять только(!) состояние пианино. Состояние пианино это массив из объектов клавиша, у каждой из которых есть несколько свойств.
Далее состояние пианино передаётся в функцию воспроизведения звука. Далее состояние пианино передаётся в отдельную функцию управления CSS классами.
Кратко так. На эту тему есть много хороших лекций. Для завершения прототипа достаточно первого совета. Для серьёзного программирования без (2) и (3) не обойтись.
Удачи!