0

const regKeys = document.querySelectorAll('.regKey');
const regCscale = ['c', 'd', 'e', 'f', 'g', 'a', 'b'];
const output = document.querySelector('.output');

function pattern(scale) {
    scale.forEach((key) => {
        key.addEventListener('click', () => {
            if (output.textContent.length >= 1) {
                output.innerHTML = '';
            }
            for (let i = 0; i < 8; i++) {
                let select = Math.round(Math.random() * 6);
                let note = scale[select];
                output.innerHTML += note + ', ';
            }
        });
    });
}

pattern(regCscale);
<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Title</title>
    <link rel="stylesheet" href="index.css">
</head>
<body>

<h1 class="title">Melody Help</h1>

<div class="reg-scales">
    <div class="c-scale regKey">C</div>
    <div class="c#-scale regKey">C#</div>
    <div class="d-scale regKey">D</div>
    <div class="d#-scale regKey">D#</div>
    <div class="e-scale regKey">E</div>
    <div class="f-scale regKey">F</div>
    <div class="f#-scale regKey">F#</div>
    <div class="g-scale regKey">G</div>
    <div class="g#-scale regKey">G#</div>
    <div class="a-scale regKey">A</div>
    <div class="a#-scale regKey">A#</div>
    <div class="b-scale regKey">B</div>
</div>

<div class="output"></div>

<script src="index.js"></script>
</body>
</html>

I've tried to find help similar to my situation but I can't find one. Im making a program that generates different notes of the piano based on scales and I can't pass the array of the different scales into my function.

Barmar
  • 741,623
  • 53
  • 500
  • 612
ssethzz
  • 43
  • 5
  • 4
    Are you calling `addEventListener` on strings? – Guerric P Aug 16 '21 at 21:49
  • Like @GuerricP said, you are adding an eventlistene to strings. You could add matching IDs to your array for each div and then go for `document.getElementById(key).addEvenetListener` if you want to keep this format. However I personally wouldnt have your approach at all – MrJami Aug 16 '21 at 21:54
  • @GuerricP No, I'm trying to start the event when one of the keys is clicked – ssethzz Aug 16 '21 at 21:55
  • @MrJami What would your approach be? – ssethzz Aug 16 '21 at 21:55
  • `output` is a single element, not a NodeList. You can't use `forEach` with it. – Barmar Aug 16 '21 at 21:56
  • Did you mean to use `pattern(regKeys)`? – Barmar Aug 16 '21 at 21:57
  • @ssethzz I know it might sound much, but I personally would work with angular. I know Angular is too much for such a small application, but because I use it daily, I feel comfortable with it. But there is for sure approaches with VanillaJS, once like I told you adding the IDs manually. Or you inject the divs via JS into your html, so you can just work with one array and dont need to write a lot – MrJami Aug 16 '21 at 21:59
  • `Math.round(Math.random() * 6)` should be `Math.floor(Math.random() * scale.length)` if you want to select a random element of the array. – Barmar Aug 16 '21 at 21:59
  • @MrJami Why do you think he's adding the event listener to strings? – Barmar Aug 16 '21 at 22:01
  • `scale[select]` is a DOM element. Why are you trying to put that in `innerHTML`? – Barmar Aug 16 '21 at 22:02
  • Maybe you should be selecting the random element from the `regCscale` array, not `scale`. – Barmar Aug 16 '21 at 22:03
  • Why are you picking random notes instead of using the note that the user clicks on? – Barmar Aug 16 '21 at 22:04
  • @Barmar because he had at first `pattern(regCscale);` and not `pattern(output);` just notices that it got edited. – MrJami Aug 16 '21 at 22:16
  • 1
    @MrJami Hmm, that was changed by someone else when they converted it to a Stack Snippet. – Barmar Aug 16 '21 at 22:17
  • @RyanWilson Why did you change `pattern(regCscale)` to `pattern(output)` when you converted to Stack Snippet? Neither of them is correct, but they're wrong for different reasons. – Barmar Aug 16 '21 at 22:18
  • @Barmar I was fiddling with the code when I turned it into a snippet for the OP. That was a typo. – Ryan Wilson Aug 17 '21 at 12:10

1 Answers1

1

You should pass regKeys to pattern, not regCscale, since it expects a list of DOM elements that it can add event listeners to.

You should select a random element of regCscale within the function, not an element of scale. See Getting a random value from a JavaScript array for the proper way to do this.

const regKeys = document.querySelectorAll('.regKey');
const regCscale = ['c', 'd', 'e', 'f', 'g', 'a', 'b'];
const output = document.querySelector('.output');

function pattern(scale) {
  scale.forEach((key) => {
    key.addEventListener('click', () => {
      if (output.textContent.length >= 1) {
        output.innerHTML = '';
      }
      for (let i = 0; i < 8; i++) {
        let select = Math.floor(Math.random() * regCscale.length);
        let note = regCscale[select];
        output.innerHTML += note + ', ';
      }
    });
  });
}

pattern(regKeys);
<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="UTF-8">
  <title>Title</title>
  <link rel="stylesheet" href="index.css">
</head>

<body>

  <h1 class="title">Melody Help</h1>

  <div class="reg-scales">
    <div class="c-scale regKey">C</div>
    <div class="c#-scale regKey">C#</div>
    <div class="d-scale regKey">D</div>
    <div class="d#-scale regKey">D#</div>
    <div class="e-scale regKey">E</div>
    <div class="f-scale regKey">F</div>
    <div class="f#-scale regKey">F#</div>
    <div class="g-scale regKey">G</div>
    <div class="g#-scale regKey">G#</div>
    <div class="a-scale regKey">A</div>
    <div class="a#-scale regKey">A#</div>
    <div class="b-scale regKey">B</div>
  </div>

  <div class="output"></div>

  <script src="index.js"></script>
</body>

</html>
Barmar
  • 741,623
  • 53
  • 500
  • 612