Improving back/home button added to manageCover

JavaScript
1

How it works is, the back/home button allows you to go back to the previous page.

One of these would be clicked.

To go back to the previous page, this would be clicked.

How can the back/home button be improved?
https://jsfiddle.net/4m7yborc/

What would also need to be added to the code is for the video to stop if you click the home button while the video is playing.

Currently, if the video is playing and you click the home button, the video doesn’t shut off, it just keeps playing.

back/home button code:

function homeClickHandler(evt) {
    const home = evt.currentTarget;
    showHome(home);
  }

  function showHome() {
    const theActive = document.querySelector(".with-curtain.active");
    const theHides = document.querySelectorAll(".hide");
    const theBody = document.querySelector("body");
    theActive.classList.remove("active");
    theHides.forEach(function(removeHide) {
      removeHide.classList.remove("hide");
    });

    void theBody.offsetWidth; //restart animation
    theBody.classList.toggle("bodytoggle");
  }

  function addClickToHome(goHome) {
    goHome.forEach(function addEventHandler(goHome) {
      goHome.addEventListener("click", homeClickHandler);
    });
  }

This was added:

    const goHome = document.querySelectorAll('.home');
    addClickToHome(goHome);

To here:

  function init(selectors) {
    config.containers = document.querySelectorAll(selectors.container);
    const playButtons = document.querySelectorAll(selectors.playButton);
    addClickToButtons(playButtons);
    const goHome = document.querySelectorAll('.home');
    addClickToHome(goHome);
  }

Full Code:

const manageCover = (function makeManageCover() {
    const config = {};

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

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

    function hideAll(elements) {
        elements.forEach(hide);
    }

    function showCovers(playButton) {
        const cover = playButton.parentElement;
        cover.classList.add("active");
        show(cover);
    }

    function coverClickHandler(evt) {
        hideAll(config.containers);
        const cover = evt.currentTarget;
        showCovers(cover);
    }

    function homeClickHandler(evt) {
        const home = evt.currentTarget;
        showHome(home);
    }

    function showHome() {
        const theActive = document.querySelector(".with-curtain.active");
        const theHides = document.querySelectorAll(".hide");
        const theBody = document.querySelector("body");
        theActive.classList.remove("active");
        theHides.forEach(function (removeHide) {
            removeHide.classList.remove("hide");
        });

        void theBody.offsetWidth; //restart animation
        theBody.classList.toggle("bodytoggle");

    }

    function addClickToButtons(playButtons) {
        playButtons.forEach(function addEventHandler(playButton) {
            playButton.addEventListener("click", coverClickHandler);
        });
    }

    function addClickToHome(goHome) {
        goHome.forEach(function addEventHandler(goHome) {
            goHome.addEventListener("click", homeClickHandler);
        });
    }

    function addCoverHandler(coverSelector, handler) {
        const cover = document.querySelector(coverSelector);
        cover.addEventListener("click", handler);
    }


    function init(selectors) {
        config.containers = document.querySelectorAll(selectors.container);
        const playButtons = document.querySelectorAll(selectors.playButton);
        addClickToButtons(playButtons);
        const goHome = document.querySelectorAll(".home");
        addClickToHome(goHome);

    }

    return {
        addCoverHandler,
        init
    };
}());


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

It seems that there is a rather obvious solution to this, that being to add to the manageCover code a method called addCloseHandler similar to the addCoverHandler, so that you can later on use manageCover.addCloseHandler to give it a function that stops the player.

1 Like
3

Yes, how would I add that to the code? or, should that be the last thing worked on?

Because, in jslint I get a warning message.
https://jsfiddle.net/4m7yborc/

javascript

const theBody = document.querySelector("body");

void theBody.offsetWidth; //restart animation
theBody.classList.toggle("bodytoggle");
}

css

.bodytoggle {
  animation: fade2 2s ease forwards;
}

To do the stop video thing, I think this would be used.

player.stopVideo();

Which would stop the video when the back/home button is pressed.

That is what I am thinking.

In terms of the addCloseHandler, something similar to this is what you mean:

    function addCloseHandler( closeSelector, handler) {
        const close = document.querySelector( closeSelector);
        close.addEventListener("click", handler);
    }

I think the unexpected ‘void’ issue should be addressed first if you agree.

And anything else that needs to be fixed in regards to the back/home button.
I’m not sure how much of that needs to be redone or adjusted.

4

The reason for the void offsetWidth is to touch the DOM so that the animation triggers.
That is called “a hack” and is a bad way to do things.

The DOM is already touched (and actually modified too) when the class is toggled in the next line, so the void line can be completely removed.

1 Like
5

Thank you for bringing that to my attention.

Removed and fixed.

Is there anything else that should be changed or adjusted?

Question, the showHome() function is not using any variables, should it be?

or should it stay left empty? Maybe one is not needed.

Also,

showHome should come before homeClickHandler I believe.
https://jsfiddle.net/y1jeao5c/1/

Do I have all of these functions organized in the right order?

I added in addCloseHandler but it is not yet set up in the code. That is something I need help with.

  function showCovers(playButton) {
    const cover = playButton.parentElement;
    cover.classList.add("active");
    show(cover);
  }

  function coverClickHandler(evt) {
    hideAll(config.containers);
    const cover = evt.currentTarget;
    showCovers(cover);
  }

  function showHome() {
    const theActive = document.querySelector(".with-curtain.active");
    const theHides = document.querySelectorAll(".hide");
    const theBody = document.querySelector("body");
    theActive.classList.remove("active");
    theHides.forEach(function(removeHide) {
      removeHide.classList.remove("hide");
    });

    theBody.classList.toggle("bodytoggle");
  }

  function homeClickHandler(evt) {
    const home = evt.currentTarget;
    showHome(home);
  }

  function addClickToButtons(playButtons) {
    playButtons.forEach(function addEventHandler(playButton) {
      playButton.addEventListener("click", coverClickHandler);
    });
  }

  function addClickToHome(goHome) {
    goHome.forEach(function addEventHandler(goHome) {
      goHome.addEventListener("click", homeClickHandler);
    });
  }

  function addCloseHandler(closeSelector, handler) {
    const close = document.querySelector(closeSelector);
    close.addEventListener("click", handler);
  }

  function addCoverHandler(coverSelector, handler) {
    const cover = document.querySelector(coverSelector);
    cover.addEventListener("click", handler);
  }

  function init(selectors) {
    config.containers = document.querySelectorAll(selectors.container);
    const playButtons = document.querySelectorAll(selectors.playButton);
    addClickToButtons(playButtons);
    const goHome = document.querySelectorAll(".home");
    addClickToHome(goHome);

  }

  return {
    addCoverHandler,
    init
  };
}());

Order 1
https://jsfiddle.net/y1jeao5c/1/

function showCovers(playButton) 
function coverClickHandler(evt)
function showHome() 
function homeClickHandler(evt)
function addClickToButtons(playButtons)

function addClickToHome(goHome)
function addCloseHandler(closeSelector, handler)
function addCoverHandler(coverSelector, handler)
function init(selectors)

Order 2

I think this way.
https://jsfiddle.net/2x8y5kwh/

function showCovers(playButton)
function coverClickHandler(evt) 
function showHome()
function homeClickHandler(evt)
function addClickToButtons(playButtons)

function addCoverHandler(coverSelector, handler)
function addClickToHome(goHome)
function addCloseHandler(closeSelector, handler)
function init(selectors)

My thinking:

These Come first:

function showCovers(playButton)
function coverClickHandler(evt)

These come next:

function showHome()
function homeClickHandler(evt)

Back to the beginning: this is next
function addClickToButtons(playButtons)

Followed by:

function addCoverHandler(coverSelector, handler)
function addClickToHome(goHome)
function addCloseHandler(closeSelector, handler)
function init(selectors)
6

How is the manageCover code responsible for the close and home stuff? None of that belong in there, and should be in a different section, likely called interface or more specifically UI which I like to instead call chrome.

1 Like
7

I removed all the home/close stuff, but I don’t know where it is being moved to, and how.
https://jsfiddle.net/1jac0yqe/

Would I be placing it all inside a new function?

I don’t think you meant for the function name to be called interface, or maybe you did.

I changed it to userInterface()

(function userInterface() {

function showHome() {
    const theActive = document.querySelector(".with-curtain.active");
    const theHides = document.querySelectorAll(".hide");
    const theBody = document.querySelector("body");
    theActive.classList.remove("active");
    theHides.forEach(function(removeHide) {
      removeHide.classList.remove("hide");
    });

    theBody.classList.toggle("bodytoggle");
  }

  function homeClickHandler(evt) {
    const home = evt.currentTarget;
    showHome(home);
  }

  function addClickToHome(goHome) {
    goHome.forEach(function addEventHandler(goHome) {
      goHome.addEventListener("click", homeClickHandler);
    });
  }

  function addCloseHandler(closeSelector, handler) {
    const close = document.querySelector(closeSelector);
    close.addEventListener("click", handler);
  }

  function init(selectors) {
    const goHome = document.querySelectorAll(".home");
    addClickToHome(goHome);
  }

  return {
    addCloseHandler,
    init
  };
}());
8

Yes, userInterface works. There should be no direct connection between userInterface and manageCover. That way they remain nice and easy to manage from a set of code external to them instead.

1 Like
9

Where should function userInterface() be placed within here?

Is there a specific location where it would go?

I placed it right below manageCover
https://jsfiddle.net/zk68ashu/

After that, if that’s where it would be placed, I don’t know how to get it to work.

const manageCover = (function makeManageCover() {
  const config = {};

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

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

  function hideAll(elements) {
    elements.forEach(hide);
  }

  function showCovers(playButton) {
    const cover = playButton.parentElement;
    cover.classList.add("active");
    show(cover);
  }

  function coverClickHandler(evt) {
    hideAll(config.containers);
    const cover = evt.currentTarget;
    showCovers(cover);
  }

  function addClickToButtons(playButtons) {
    playButtons.forEach(function addEventHandler(playButton) {
      playButton.addEventListener("click", coverClickHandler);
    });
  }

  function addCoverHandler(coverSelector, handler) {
    const cover = document.querySelector(coverSelector);
    cover.addEventListener("click", handler);
  }

  function init(selectors) {
    config.containers = document.querySelectorAll(selectors.container);
    const playButtons = document.querySelectorAll(selectors.playButton);
    addClickToButtons(playButtons);
  }

  return {
    addCoverHandler,
    init
  };
}());

(function userInterface() {

function showHome() {
    const theActive = document.querySelector(".with-curtain.active");
    const theHides = document.querySelectorAll(".hide");
    const theBody = document.querySelector("body");
    theActive.classList.remove("active");
    theHides.forEach(function(removeHide) {
      removeHide.classList.remove("hide");
    });

    theBody.classList.toggle("bodytoggle");
  }

  function homeClickHandler(evt) {
    const home = evt.currentTarget;
    showHome(home);
  }

  function addClickToHome(goHome) {
    goHome.forEach(function addEventHandler(goHome) {
      goHome.addEventListener("click", homeClickHandler);
    });
  }

  function addCloseHandler(closeSelector, handler) {
    const close = document.querySelector(closeSelector);
    close.addEventListener("click", handler);
  }

  function init(selectors) {
    const goHome = document.querySelectorAll(".home");
    addClickToHome(goHome);
  }

  return {
    addCloseHandler,
    init
  };
}());

function combinePlayerOptions(options1 = {}, options2 = {}) {
  const combined = Object.assign({}, options1, options2);
  Object.keys(options1).forEach(function checkObjects(prop) {
    if (typeof options1[prop] === "object") {
      combined[prop] = Object.assign({}, options1[prop], options2[prop]);
    }
  });
  return combined;
}

const videoPlayer = (function makeVideoPlayer() {
10

Usually the user interface is more fundamental and higher up than other things. It can set up events on the page, so that when the close event occurs, certain things can be attached to that event. Or when the home is clicked, other things can be attached to that event too.

1 Like
11

I don’t believe that would be enough to trigger the animation as adding and removing a class doesn’t trigger a reflow of the page.

However the void offset width can be removed in that specific code because the animation was changed instead to use two animations instead of one.

body {
  background: #353198;
  animation: fade 2s ease 0s forwards;
}

.bodytoggle {
  animation: fade2 2s ease forwards;
}

@keyframes fade {
  0% {
    opacity: 0;
  }

  100% {
    opacity: 1;
  }
}

@keyframes fade2 {
  0% {
    opacity: 0;
  }

  100% {
    opacity: 1;
  }
}

The issue does not arise in the above code even though the animations are doing the same thing.

Again that can be considered a bit of a hack because we should have been able to use the same animation name for both.and not needed two keyframes.

1 Like
12

That’s odd - I thought that by toggling the “active” class and showing the element, that it would result in some kind of reflow occuring.

    cover.classList.add("active");
    show(cover);

Was I mistaken about that?

13

I think it depends whether you are changing the element that needs the animation to run. i.e. The body element in this case. Toggling a class doesn’t work to re-trigger the animation unless you add a setTimeout delay but the delay you need varies between browsers so its not 100% safe.

I’m just off out at the moment otherwise I would put up a small demonstration as I’m sure its something you could solve more efficiently.:slight_smile:

1 Like
15

From your Post #2

void theBody.offsetWidth;

Was used with 2 keyframes/animations, not 1.

In saying that, void theBody.offsetWidth; was never used with 1 keyframe.

Am I missing something here?

body {
  background: #353198;
  animation: fade 2s ease 0s forwards;
}

.bodytoggle {
  animation: fade2 2s ease forwards;
}

@keyframes fade {
  0% {
    opacity: 0;
  }
  100% {
    opacity: 1;
  }
}

@keyframes fade2 {
  0% {
    opacity: 0;
  }
  100% {
    opacity: 1;
  }
}
17

No that JS code came from way back before that (I think) and has probably been in your code since early on when you asked for the body to fade but things changed along the way. Its another case of you asking for one thing and then trying to do another with the same code so there are bits of left over code everywhere.:slight_smile:

1 Like
18

void theBody.offsetWidth; Has only been used ever 1 time on here.

From where you added it. Post #2

That was the 1st time it had ever been used.

Where was it ever removed from the code in that post thread?

782×528 29.6 KB

19

I’m sure we have used it before but it may just have been when I was messing around with the file locally. I must have added it initially then changed the css instead so the js became redundant. I tend not to let other people’s code stay in my head especially when they chop and change so much. As I keep saying that’s the problem with jumping from one thing to another rather than building with purpose.

You can see from the CSS tricks link I posted above when that JS may be needed but it’s better to use the CSS workarounds where possible.

1 Like
20

I changed it to manageUI instead, if that is good.

const manageUI = (function makeManageUI() {

What is supposed to go inside here?

Right now the code is working with it empty.

  manageUI.init({

  });
}

I think it is allowed to be empty, right.

It would be written like this, or would something go inside it?
manageUI.init({});

I got the code working here:
Meaning, the home/close button is working.
https://jsfiddle.net/awy26kon/

What needs to be changed or adjusted in the code?

What still needs to be added is for the video to stop, when home is clicked, but I don’t think I am up to that part yet. player.stopVideo();

Which is what this function is supposed to help with doing.

    function addCloseHandler(closeSelector, handler) {
        const close = document.querySelector(closeSelector);
        close.addEventListener("click", handler);
    }

manageUI Code:

const manageUI = (function makeManageUI() {

    function showHome() {
        const theActive = document.querySelector(".with-curtain.active");
        const theHides = document.querySelectorAll(".hide");
        const theBody = document.querySelector("body");
        theActive.classList.remove("active");
        theHides.forEach(function (removeHide) {
            removeHide.classList.remove("hide");
        });

        theBody.classList.toggle("bodytoggle");
    }

    function homeClickHandler(evt) {
        const home = evt.currentTarget;
        showHome(home);
    }

    function addClickToHome(goHome) {
        goHome.forEach(function addEventHandler(goHome) {
            goHome.addEventListener("click", homeClickHandler);
        });
    }

    function addCloseHandler(closeSelector, handler) {
        const close = document.querySelector(closeSelector);
        close.addEventListener("click", handler);
    }

    function init() {
        const goHome = document.querySelectorAll(".home");
        addClickToHome(goHome);
    }

    return {
        addCloseHandler,
        init
    };
}());

manageUI.init({});
21

What should I do next?

22

You should check that the manageUI can do certain things, such as to show the home button, hide the home button, show the exit button, hide the exit button. After that you can set it up to listen for home events and exit events. That then lets other code add home events and exit events to achieve those things too.