Combining 3 functions into a single function

Instead of using:

.jacket-left, .jacket-middle, jacket-right.

I would just be using .jacket

To target all 3 covers.

How would this be written into the code?
const cover = document.querySelectorAll(".jacket");

The original code

(function manageCover() {
  "use strict";

  function hide(el) {
    el.classList.add("hide");
  }

  function coverClickHandler(evt) {
    const cover = evt.currentTarget;
    hide(cover);
  }
  
  const cover = document.querySelector(".jacket-left");
  cover.addEventListener("click", coverClickHandler);
}());


(function manageCover() {
  "use strict";

  function hide(el) {
    el.classList.add("hide");
  }

  function coverClickHandler(evt) {
    const cover = evt.currentTarget;
    hide(cover);
  }
  const cover = document.querySelector(".jacket-middle");
  cover.addEventListener("click", coverClickHandler);
}());


(function manageCover() {
  "use strict";

  function hide(el) {
    el.classList.add("hide");
  }

  function coverClickHandler(evt) {
    const cover = evt.currentTarget;
    hide(cover);
  }
  const cover = document.querySelector(".jacket-right");
  cover.addEventListener("click", coverClickHandler);
}());

Update*
I just noticed I could use const here instead of let.
for (let cover of covers) {

for…of

Way 1.) for…of

const covers = document.querySelectorAll('.jacket')

for (let cover of covers) {
    cover.addEventListener('click', (evt) => {
        evt.currentTarget.classList.add('hide')
    })
}

Way 2.) for…of

const covers = document.querySelectorAll('.jacket')

for (let cover of covers) {
    cover.addEventListener('click', function (evt) {
        evt.currentTarget.classList.add('hide')
    })
}

Way 3.) for…of

const covers = document.querySelectorAll('.jacket-left, .jacket-middle, .jacket-right')

for (let cover of covers) {
    cover.addEventListener('click', (evt) => {
        evt.currentTarget.classList.add('hide')
    })
}

Way 4.) for…of

const covers = document.querySelectorAll('.jacket-left, .jacket-middle, .jacket-right')

for (let cover of covers) {
    cover.addEventListener('click', function (evt) {
        evt.currentTarget.classList.add('hide')
    })
}

.forEach

Way 1.) .forEach

const covers = document.querySelectorAll(".jacket");
covers.forEach((cover) => {
    cover.addEventListener("click", coverClickHandler);
});

Way 2.) .forEach

const covers = document.querySelectorAll(".jacket");
covers.forEach(function (cover) {
    cover.addEventListener("click", coverClickHandler);
});

Way 3.) .forEach
https://jsfiddle.net/jy1Lewhg/3/

const covers = document.querySelectorAll(".jacket-left, .jacket-middle, .jacket-right");
covers.forEach((cover) => {
    cover.addEventListener("click", coverClickHandler);
});

Way 4.) .forEach
https://jsfiddle.net/jy1Lewhg/4/

const covers = document.querySelectorAll(".jacket-left, .jacket-middle, .jacket-right");
covers.forEach(function (cover) {
    cover.addEventListener("click", coverClickHandler);
});

forEach version 4 as a named function, is the preferred technique.

const covers = document.querySelectorAll(".jacket-left, .jacket-middle, .jacket-right");
covers.forEach(function addCoverListener(cover) {
    cover.addEventListener("click", coverClickHandler);
});

The named function can be easily extracted out too.

function addCoverListener(cover) {
    cover.addEventListener("click", coverClickHandler);
}
const covers = document.querySelectorAll(".jacket-left, .jacket-middle, .jacket-right");
covers.forEach(addCoverListener);

2 Likes

What’s the reason why you used:

.jacket-left, .jacket-middle, .jacket-right

Instead of just .jacket?

function addCoverListener(cover) {
    cover.addEventListener("click", coverClickHandler);
}
const covers = document.querySelectorAll(".jacket");
covers.forEach(addCoverListener);

My focus has been on the style of for loop. Not on fixing all of your problems.
I plan to avoid investing a lot of time with you, due to strong conflicts in how I feel about your project.

For the record though I just copied the complete code block from your “Way 4.)” example.

I was just curious why you chose to work with 3 classes instead of just the one .class.

In the html it’s written like this.

    <div class="jacket jacket-left">
    <div class="jacket jacket-middle">
    <div class="jacket jacket-right">

Because, I thought less characters on a stribg is better to work with.

I didn’t make that choice.

1 Like

Either way would work fine then, no difference.

function addCoverListener(cover) {
    cover.addEventListener("click", coverClickHandler);
}
const covers = document.querySelectorAll(".jacket-left, .jacket-middle, .jacket-right");
covers.forEach(addCoverListener);
function addCoverListener(cover) {
    cover.addEventListener("click", coverClickHandler);
}
const covers = document.querySelectorAll(".jacket");
covers.forEach(addCoverListener);

Incorrect.

There are several differences, but you have the remarkable talent of rubbing me the wrong way that I want to spend as little time with you as possible.

Why is that incorrect?

3 Classes

function addCoverListener(cover) {
    cover.addEventListener("click", coverClickHandler);
}
const covers = document.querySelectorAll(".jacket-left, .jacket-middle, .jacket-right");
covers.forEach(addCoverListener);

}());

1 Class

function addCoverListener(cover) {
    cover.addEventListener("click", coverClickHandler);
}
const covers = document.querySelectorAll(".jacket");
covers.forEach(addCoverListener);

}());

How would I be able to write the code this way?

The reason why this was done was to make the string shorter.

How it was originally written:

    function hideAllButtons(button) {
        var buttonSelectors = ".play, .pause, .initial, .speaker";
        button.querySelectorAll(buttonSelectors).forEach(hide);
    }

All I would be doing is.

Changing this:

The way it’s written like this is over 80 characters long.

const covers = document.querySelectorAll(".jacket-left, .jacket-middle, .jacket-right");
covers.forEach(addCoverListener);

To this:

const covers = ".jacket-left, .jacket-middle, .jacket-right";
covers.querySelectorAll(covers).forEach(addCoverListener);

My attempts

Not working:

function addCoverListener(cover) {
    cover.addEventListener("click", coverClickHandler);
}
 const covers = ".jacket-left, .jacket-middle, .jacket-right";
covers.querySelectorAll(covers).forEach(addCoverListener);

}());

Not working:

function addCoverListener(covers) {
    covers.addEventListener("click", coverClickHandler);
}
 const covers = ".jacket-left, .jacket-middle, .jacket-right";
covers.querySelectorAll(covers).forEach(addCoverListener);

}());

Update***

I got it working like this.
Is that good?

or should it be written a different way?

function addCoverListener(covers) {
    covers.addEventListener("click", coverClickHandler);
}
 const covers = ".jacket-left, .jacket-middle, .jacket-right";
document.querySelectorAll(covers).forEach(addCoverListener);
}());

If it used to look like this:
Then I did it right, right?

const covers = document.querySelectorAll(".jacket-left, .jacket-middle, .jacket-right");
covers.forEach(addCoverListener);

For me, I wouldn’t assign selectors to a variable like that. Instead, I would assign the result of querySelectorAll to the variable.

const covers = document.querySelectorAll(".jacket-left, .jacket-middle, .jacket-right");

If you consider that line to be too long, that is when the selectors are appropriately moved to a separate variable. The important thing is that the covers variable remains as a reference to the elements found by querySelectorAll too.

const coversSelector = ".jacket-left, .jacket-middle, .jacket-right";
const covers = document.querySelectorAll(coversSelector);

Or, you can adjust other parts of the HTML code so that only a single selector is needed.

The covers don’t stay hidden that way.

const coversSelector = ".jacket-left, .jacket-middle, .jacket-right";
const covers = document.querySelectorAll(coversSelector);

They are hidden here.

const covers = ".jacket-left, .jacket-middle, .jacket-right";
document.querySelectorAll(covers).forEach(addCoverListener);

In earlier assistance I was giving you many months ago, you made it very clear that you were taking audio streams you don’t own, and were publishing them with your own visuals, expressly against the terms and conditions of where you were taking them from.

Because of that, I am seriously conflicted about whether to help you any more with your project.

I don’t think that I can help you any further.

1 Like

This code specifically deals with YouTube.

There isn’t anything in here that deals with audio streams.

const load = (function makeLoad() {
    "use strict";

    function _load(tag) {
        return function (url) {
            return new Promise(function (resolve) {
                const element = document.createElement(tag);
                const parent = "body";
                const attr = "src";
                element.onload = function () {
                    resolve(url);
                };
                element[attr] = url;
                document[parent].appendChild(element);
            });
        };
    }
    return {
        js: _load("script")
    };
}());

(function manageCover() {
    "use strict";

    function hide(el) {
        el.classList.add("hide");
    }

    function coverClickHandler(evt) {
        const cover = evt.currentTarget;
        hide(cover);
    }

function addCoverListener(covers) {
    covers.addEventListener("click", coverClickHandler);
}
const coversSelector = ".jacket-left, .jacket-middle, .jacket-right";
const covers = document.querySelectorAll(coversSelector);
}());

const videoPlayer = (function makeVideoPlayer() {
    "use strict";
    const players = [];

    function onPlayerReady(event) {
        const player = event.target;
        player.setVolume(100); // percent
    }

    let hasShuffled = false;

    function onPlayerStateChange(event) {
        const player = event.target;
        if (!hasShuffled) {
            player.setShuffle(true);
            player.playVideoAt(0);
            hasShuffled = true;
        }
        if (event.data === YT.PlayerState.PLAYING) {
            for (let i = 0; i < players.length; i++) {
                if (players[i] !== event.target) players[i].pauseVideo();
            }
        }

        const playerVars = player.b.b.playerVars;
        if (playerVars.loop && event.data === YT.PlayerState.ENDED) {
            player.seekTo(playerVars.start);
        }
    }

    function addVideo(video, settings) {
        players.push(new YT.Player(video, Object.assign({
            videoId: video.dataset.id,
            host: "https://www.youtube-nocookie.com",
            events: {
                "onReady": onPlayerReady,
                "onStateChange": onPlayerStateChange
            }
        }, settings)));
    }

    function init(video, settings) {
        load.js("https://www.youtube.com/player_api").then(function () {
            YT.ready(function () {
                addVideo(video, settings);
            });
        });
    }
    return {
        init
    };
}());

function loadPlayer(opts) {
    "use strict";

    function show(el) {
        el.classList.remove("hide");
    }

    function initPlayer(wrapper) {
        const video = wrapper.querySelector(".video");
        opts.width = opts.width || 198;
        opts.height = opts.height || 198;
        opts.autoplay = 1;
        opts.controls = 1;
        opts.rel = 0;
        opts.iv_load_policy = 3;
        opts.fs = 0;
        opts.disablekb = 1;

        function paramInOpts(settings, param) {
            if (opts[param] !== undefined) {
                settings[param] = opts[param];
            }
            return settings;
        }
        const settingsParams = ["width", "height", "videoid", "host"];
        const settings = settingsParams.reduce(paramInOpts, {});
        const playerVarsParams = ["autoplay", "cc_load_policy",
            "controls", "disablekb", "end", "fs", "iv_load_policy",
            "list", "listType", "loop", "playlist", "rel", "start"
        ];
        settings.playerVars = playerVarsParams.reduce(paramInOpts, {});
        videoPlayer.init(video, settings);
    }

    function coverClickHandler(evt) {
        const wrapper = evt.currentTarget.nextElementSibling;
        show(wrapper);
        initPlayer(wrapper);
    }
    const cover = document.querySelector(opts.target);
    cover.addEventListener("click", coverClickHandler);
}
const playlist = "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g";

loadPlayer({
    target: ".jacket-left",
    width: 277,
    height: 207
});

loadPlayer({
    target: ".jacket-middle",
    width: 277,
    height: 207
});
loadPlayer({
    target: ".jacket-right",
    width: 277,
    height: 207
});


loadPlayer({
    target: ".jacketc",
    width: 600,
    height: 338,
    loop: true,
    playlist
});
loadPlayer({
    target: ".alpha",
    start: 0,
    end: 280,
    loop: true
});
loadPlayer({
    target: ".beta",
    start: 0,
    end: 240,
    loop: true
});
loadPlayer({
    target: ".gamma",
    start: 0,
    end: 265,
    loop: true
});
loadPlayer({
    target: ".delta",
    start: 4,
    end: 254,
    loop: true
});
loadPlayer({
    target: ".epsilon",
    start: 0,
    end: 242,
    loop: true
});
loadPlayer({
    target: ".zeta",
    start: 0,
    end: 285,
    loop: true
});
loadPlayer({
    target: ".eta",
    start: 23,
    end: 312,
    loop: true
});
loadPlayer({
    target: ".theta",
    start: 2
});
loadPlayer({
    target: ".iota"
});

That sadly doesn’t change how I feel about your approach to things, which is why I cannot in good faith help you any further.

1 Like

But it is part of the same project, is it not?

II have multiple different iterations of the code made.

One that uses only audio.

One with a bunch of just videos.

I made multiple different variations of them.

But this code I’m working on only deals with YouTube, and not other things.

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.