Changes to cope with separating HTML players and covers

Sorry but no, the extra stuff you added is completely the wrong approach.

I know that you’re keen to make progress on this, as are we all, but attempting to go fast doesn’t actually achieve that, and instead only slows everything down instead.

Robert C. Martin wrote about this recently in his book The Clean Coder where in one part he talks about problem of coding to a deadline and rushing things, and the fallacy of working overtime.

“There is no way to rush. You can’t make yourself code faster. You can’t make yourself solve problems faster. If you try, you’ll just slow yourself down and make a mess that slows everything else down too.”

You now need to remove all that you did as none of it is usable.

Let me know when you have done what has been asked of you in post #47 and further progress can then be made from there.

Create a players array at the top of the manageUI function

This is wrong?

const players = {} is a players array

‘players’ has already been declared

I don’t understand then.

const players = {}

const manageUI = (function makeManageUI() {

Yes, that is wrong. You have created an object there. An object is not an array. You create an array using []’instead.

You hav3 placed it in the wrong place. I think that I said that it goes at the top of the function. You have placed it above the function instead.

Above, top, bottom, and below all have different meanings when it comes to functions. The top of a function means inside of the function, above all of the other things inside of that function.

All of these are at the top, should these be in a different order?

or is this good? https://jsfiddle.net/o3vzsp05/

const manageUI = (function makeManageUI() {
  const body = document.body;

  const players = [];

  function getWrapper(cover) {
    const parent = cover.parentElement;
    const wrapper = parent.querySelector(".wrap");
    return wrapper
  }

Next is:

add a function call to findPlayers();

I don’t understand that instruction.

Like This? https://jsfiddle.net/p521qmzr/

Can progress be made from here?

 findPlayers = function(){};

  function init() {
    manageCover.init({
      container: ".container",
      playButton: ".thePlay"
    });

The locations are good as they are.

Imagine that a function already exists called findPlayers. At the top of the init function (not above it) invoke that findPlayers function, as you would when using any other function.

When you have done that and you are getting a suitable error that the function doesn’t exist, we can then create the findPlayers function above the other functions in the manageUI code.

This is what I did: https://jsfiddle.net/ukzveory/1/

Did I do something wrong?

Did I do this instruction wrong?:

create the findPlayers function above the other functions in the manageUI code.

I am getting this error:

findPlayers is not defined

It says it still doesn’t exist.

const manageUI = (function makeManageUI() {
  const body = document.body;
  
    function findPlayers() {

  }

  const players = [];

  function getWrapper(cover) {
    const parent = cover.parentElement;
    const wrapper = parent.querySelector(".wrap");
    return wrapper
  }

  return {
    addExitHandlers,
    findPlayers,
    getWrapper,
    init
  };
}());
const players = (function coverUIPlayerFacade() {

  function addPlayer(coverSelector, playerOptions) {
    const cover = document.querySelector(coverSelector);
    const wrapper = manageUI.getWrapper(cover);
    const callback = managePlayer.adder(wrapper, playerOptions);
    manageCover.addCoverHandler(coverSelector, callback);
  }

  function init() {
    findPlayers();
    manageCover.init({
      container: ".container",
      playButton: ".thePlay"
    });

    manageUI.init({});
    manageUI.addExitHandlers(managePlayer.removePlayerHandler);
  }

  return {
    add: addPlayer,
    init
  };
}());
  1. The findPlayers function shouldn’t be above the definition of the players variable. The findPlayers function should be below it instead.

  2. You have added a reference to findPlayers in the manageUI return object. That is not suitable because there is no good reason to make it available to anything outside of the manageUI code. Remove findPlayers from that return object.

  3. Lastly, the init function is still failing to invoke the findPlayers function.

Get all of those issues dealt with and we will be back on track.

At this link I am up to 3: https://jsfiddle.net/n5brauzg/

I am stuck on it.

At a guess I’m thinking that it might be terminology that you are stuck on when it comes to the meaning of invoke, so let’s use an example by comparison.

Somewhere above the init function is another function called exitClickHandler.

  function exitClickHandler() {
    resetPage();
  }

That exitClickHandler function invokes the resetPage function.

Is that the kind of help that you need?

1 Like

I did this: https://jsfiddle.net/0xd534be/

const players = (function coverUIPlayerFacade() {

  function addPlayer(coverSelector, playerOptions) {
    const cover = document.querySelector(coverSelector);
    const wrapper = manageUI.getWrapper(cover);
    const callback = managePlayer.adder(wrapper, playerOptions);
    manageCover.addCoverHandler(coverSelector, callback);
  }

  function findPlayers() {}

  function init() {
    findPlayers();
    manageCover.init({
      container: ".container",
      playButton: ".thePlay"
    });

    manageUI.init({});
    manageUI.addExitHandlers(managePlayer.removePlayerHandler);
  }

  return {
    add: addPlayer,
    init
  };
}());

I don’t see that being done anywhere in the manageUI code. That’s where it needs to be done.

Is this what you wanted me to do? https://jsfiddle.net/rf8g1btq/

const manageUI = (function makeManageUI() {
  const body = document.body;

  const players = [];

  function findPlayers() {
    findPlayers();
  }

  function getWrapper(cover) {
    const parent = cover.parentElement;
    const wrapper = parent.querySelector(".wrap");
    return wrapper
  }

You have some something extremely silly by having findPlayers invoke itself. That will never end if anything invokes the findPlayers function.

That invoking of the findPlayers function needs to be moved down to the manageUI’s init function.

Like this? https://jsfiddle.net/a2rdz4kf/

Top

const manageUI = (function makeManageUI() {
  const body = document.body;

  const players = [];

  function findPlayers() {
 
  }

  function getWrapper(cover) {
    const parent = cover.parentElement;
    const wrapper = parent.querySelector(".wrap");
    return wrapper
  }

Bottom

function init() {
    findPlayers();
    const exitButtons = document.querySelectorAll(".exit");
    addClickToExit(exitButtons);
    body.addEventListener("animationend", animationEndHandler);
  }

Good one. We can now move on to populating the players array, after which getWrapper can then be updated to search for that cover in the players array and return the appropriate wrapper instead.

The findPlayers function will populated the players array. How that is done is by making a list of covers, a list of wrappers, and then combining each of them in a loop into an object, which we add to the players array.

So first, in the findPlayers function use document.querySelectorAll to search for “.thePlay” and assign them to a variable called allCovers. That is done because all of your elements that are covers have a consistent classname called thePlay.

This is a good time to think about renaming thePlay in your HTML and CSS to be cover instead.

Then, also in the findPlayers function, use document.querySelectorAll to search for “.wrap” which are your wrappers, and assign them to a variable called allWrappers.

You should then have two variables, one called allCovers and another called allWrappers, that each contain an array-like list of all matching HTML elements.

When you’ve achieved that I’ll take you through how to appropriately combine them together.

1 Like

Changing thePlay to cover would mean also in the javascript, right?

I did that here: https://jsfiddle.net/sd9akzx5/

  function findPlayers() {
    const allCovers = document.querySelectorAll(".cover");
    const allWrappers = document.querySelectorAll(".wrap");
  }
1 Like

Good one. We can now use forEach to loop over each of the elements in allCovers, where the looping function is called addToPlayers. The function parameters of the loop are cover and index.

We need that index reference because we are going to use it to get the same indexed item from the allWrappers array too.

Is this close? https://jsfiddle.net/oLtpsn8q/

 function findPlayers() {
    const allCovers = document.querySelectorAll(".cover");
    const allWrappers = document.querySelectorAll(".wrap");
    allWrappers.forEach(function addToPlayers(cover, index) {});
  }

Yes, that’s good.

Inside of the addToPlayers loop, push to the players array an object. That object consists of a property called cover, and a property called wrapper.

For the cover property you can use the index to reference an item from the allCovers array.
For the wrapper property you can use the index to reference an item from the allWrappers array.

I’m confused.

  function findPlayers() {
    const allCovers = document.querySelectorAll(".cover");
    const allWrappers = document.querySelectorAll(".wrap");
    allWrappers.forEach(function addToPlayers(cover, index) {
    cover(index);
    wrapper(index);
    });
  }

Like this?

     cover: index,
     wrapper: index,