Help needed understanding statements in function


#35

Hi Andres,
Thanks for helping!

I just uploaded this test page with your modifications to the script.

toggle-slide-2.html
menu-slide-toggle-2.js

Yes, the page works in Firefox since it uses display:contents;

But it suffers from needing the page reloaded in Chrome when the browser window changes width.
That's what I keeping referring back to, in how it gets resolved by targeting from the .inner div since changes in the DOM don't effect it.

The display:contents; portion of my page layout may be new to you. It was never part of the original slide toggle page you helped me with.

EDIT:
If you go back to post#26 and click on the Toggle Only link it will give a full explanation of the page layout using display:contents; and the unwrap.js script for browsers that don't support it. :slight_smile:


#36

Hey Ray, maybe we need to go and revisit the original solution after all. I haven't read this thread in its entirety, but if you can make a demo page demonstrating the problem and include a brief description of what is not working, then I can have a look for you.


#37

Post #26 has all the details you need and a link to a demo explaining the problem clearly :slight_smile:


#38

Hi Pullo,
I'm glad to see you drop in :slight_smile:

As Paul mentioned, all the details and demo are in post #26

The Toggle Only page is using the script you wrote in my other thread.
It is proof of concept that working from the .inner div is the way to handle changes in the DOM, for browsers that get the unwrap script.

The screenshots in post#26 will compliment everything that is said in my demo page.

What My Goal Is

I am now trying to get the Slide Toggle menu working in my flexbox & display:contents page layout

After reading the Toggle Only page, there is a link at the bottom that takes you to the Slide Toggle page.

There are links at the bottom of the Slide Toggle page so you can grab the files and work with them.That page has the slide toggle script that was developed earlier on in this thread.

Some modifications were made
The menu-slide-toggle-2.js seen in post #35 was Andres_Vaquero's refinement of PaulOB's script.

It is a little different from the original slide-toggle script but it still works the same.


#39

Hey man, so to summarize the issue:

On this page (http://www.rayswoodworks.com/rwd-test/js-test/toggle-slide-2.html) in Chrome, when you manually resize the browser window to under 650px, you need to refresh the page before the toggle script runs correctly.

Did I get that right?


#40

Yes, that is correct!

It is the exact same thing that we went through in the other thread.
As seen in my screenshot of Chrome there.

I'm just trying to turn it into a Slide Toggle now.


#41

And that page there is using the menu-slide-toggle-2.js which was Andres_Vaquero's refinement of PaulOB's script that I mentioned in post #38.

The Slide Toggle demo page uses menu-slide-toggle.js which is a slightly different version developed earlier on this thread.

You can use which ever one is easiest to modify


#42

I don't want to clutter up the thread but if all else fails this codepen version of mine is now working. It's not pretty and simply adds a class to the html element so that you can have different toggle events for whether display:contents is supported or not.


#43

Paul,
That version loses the menu in the left column for desktops in Firefox.
(It happens when the page loads in desktop view)

The menu also has to be left open in order to go to the sidebar for desktops

While under 650px in FF, close the menu, then drag to desktop width.

EDIT:
Working from the .inner div is the cross browser solution, whether display:contents is supported or not


#44

Sorry fixed now :slight_smile: (there;s still a logic error with the hamburger icon but I'll fix that later as I am out now).

I'll let the others take over this now anyway and stop butting in:)

I may be wrong (again) but I don't think that's the issue. The issue is that the event handlers are lost on the html elements because you change the html completely. Whether you work from the inner div or not the issue will be that the html is changed so you still have to put the eventListener back however you find it. I believe in my early version (ages ago) I cloned the html and preserved the event listeners and thus the original version should have worked ok.

Anyway I'll get out of your hair and leave it to the experts.:wink:


#45

Well, TIL that if you clone a node in vanilla JavaScript, any event listeners get nuked in the process. Woah. Seems that I've been writing too much jQuery (where of course you can preserve any listeners by passing true as an argument to .clone()).

Cloning a node copies all of its attributes and their values, including intrinsic (in–line) listeners. It does not copy event listeners added using addEventListener() or those assigned to element properties (e.g. node.onclick = fn).

MDN

Anyway, I'm supposing you don't want to use jQuery, so I went ahead and integrated your working slideToggle solution into my original script.

<!doctype html>
<html lang="en">
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <link rel="stylesheet" href="css/screen.css" media="all">
  <link rel="stylesheet" href="css/nav-menu.css" media="all">
  <title>Slide Toggle</title>
  <style>
    h1 {font-size:1.6em}
    h2 {font-size:1.3em}
    p {font-size:1.1em;}
    code {font-size:1.1em; background:#fff}
  </style>
</head>
<body>

<div class="wrap">
  <header>
    <h1>Slide Toggle<br> only works in Firefox</h1>
  </header>
  <div class="inner">
    <div class="sidebar">
      <nav class="top">
        <button class="toggle"><span>Menu</span></button>
        <ul class="menu">
          <li><a href="#">Page Link 1</a></li>
          <li><a href="#">Page Link 2</a></li>
          <li><a href="#">Page Link 3</a></li>
          <li><a href="#">Page Link 4</a></li>
          <li><a href="#">Page Link 5</a></li>
          <li><a href="#">Page Link 6</a></li>
        </ul>
      </nav>
      <div class="mid">
        <p>Mid div space-around</p>
        <p>Mid div space-around</p>
        <p>Mid div space-around</p>
      </div>
      <div class="bot">
        <p>Side Bottom</p>
        <p>Side Bottom</p>
      </div>
    </div>
    <div class="main">
      <main>
        <h2>Same Page Layout</h2>

        <p>Same flexbox layout using <code>display:contents</code> to remove the <code>.sidebar</code> div and leave it's child nodes in place.
        This happens when the screen width is 650px and under. At this time only Firefox supports <code>display:contents</code>.
        Firefox actually leaves the <code>.sidebar</code> div in the html but it basically acts as if it's not there, and there's
        been no changes to the DOM. The child nodes now act as if they were children of the <code>.inner</code> div, which was
        previously their grandparent.</p>

        <p>That is why the Slide Toggle menu works properly in Firefox right now. Other browsers require a page reload when the media query kicks in.</p>

        <h2>The Same Workaround as other page</h2>

        <p>Browsers that don't support <code>display:contents</code> get the <code>unwrap.js</code> script. The script removes the
        <code>.sidebar</code> div and appends the child nodes back into the html. The nodes are now children of the <code>.inner</code> div.
        But unlike <code>display:contents</code> explained above, the script actually DOES remove the <code>.sidebar</code> div from the html. The child
        nodes have temporarily lost their place in the DOM unless the page gets reloaded. Once again, all this happens when screen width is 650px and under.</p>

        <h2>Now we have the "Slide Toggle" menu on this page</h2>

        <p>This is where I need help to target <code>&lt;button class="toggle"&gt;</code> from the <code>.inner</code> div to begin with.
        Then locate <code>&lt;ul class="menu"&gt;</code> by way of <code>.nextElementSibling</code>.</p>

        <p>I neeed help setting up a new ClickHandler function for the "Slide Toggle" script (view page source), that targets everything by way of the <code>.inner</code> div.

        Then when the <code>.sidebar</code> div is removed by browsers that don't support <code>display:contents</code>, the slide
        toggle should still work without needing a page reload when the browser window changes widths.</p>

        <p>Anyone willing to help with this can find my files here on the <a href="http://www.rayswoodworks.com/rwd-test/js-test/">Index Page</a>,
        as well as a <a href="http://www.rayswoodworks.com/rwd-test/js-test/">zip file</a> to pull them down in one shot. <br><br> Thanks for Reading!</p>

      </main>
    </div>
  </div>
  <footer>Footer</footer>
</div>

<script>
  (function(){
    "use strict";

    function unwrap(el){
      var docFrag = document.createDocumentFragment();
      while (el.firstChild) {
        var child = el.removeChild(el.firstChild);
        docFrag.appendChild(child);
      }

      el.parentNode.replaceChild(docFrag, el);
    }

    function noDisplayContents(){
      var newEl = document.createElement('detect');
      document.body.appendChild(newEl);
      return getComputedStyle(newEl).getPropertyValue('display') === 'none';
    }

    function toggleMenuClickHandler(evt) {
      var clickedEl = evt.target;
      if(clickedEl.className.match('toggle')) {
        var handler = clickedEl;
        var menuNode = clickedEl.nextElementSibling;
        var isExpanded = menuNode.classList.toggle('expand');
        menuNode.style.height = isExpanded ? originalHeight + 'px' : 0;
        handler.classList.toggle('open');
      }
    }

    function handleViewportChange(mql) {
      if (mql.matches) {
        console.log('Viewport smaller than 651px');
        if (noDisplayContents()) sidebarParent.innerHTML = sidebarParentAltered;
        document.querySelector('.menu').style.height = 0;
      } else {
        console.log('Viewport larger than 651px');
        if (noDisplayContents()) sidebarParent.innerHTML = sidebarParentNormal;
        document.querySelector('.menu').style.height = 'auto';
        document.querySelector('.menu').classList.remove('expand');
        document.querySelector('.toggle').classList.remove('open');
      }
    }

    var inner = document.querySelector('.inner');
    var originalHeight = document.querySelector('.menu').offsetHeight;
    var mql = window.matchMedia('screen and (max-width: 650px)');

    inner.addEventListener('click', toggleMenuClickHandler);
    mql.addListener(handleViewportChange);

    if (noDisplayContents()){
      var sidebar = document.querySelector('.sidebar');
      var sidebarParent = sidebar.parentNode;

      var sidebarParentNormal = sidebarParent.innerHTML;
      unwrap(sidebar);
      var sidebarParentAltered = sidebarParent.innerHTML;
    }

    handleViewportChange(mql);
  })();

</script>
</body>
</html>

This uses event delegation, which actually seems to be the recommended method of preserving event listeners in a case like this.

HTH


#46

Nice and so much neater :slight_smile:


#47

Oh My Goodness, You nailed it Pullo :hugging:

Thank You So Much, for your help!

I was about to concede that this was going to be too much trouble.

It's working fine in IE11 too

Slide Toggle - Flexbox and Display:Contents


#48

No worries. Happy to help :slight_smile:


#49

@PaulOB

I want to thank you too :slight_smile:
You hung in there with me like a trooper

@Andres_Vaquero, thank you too for your contributions


#50

Yes I enjoyed the practice - sorry if I delayed you from getting the real answer :slight_smile:

Just for posterity I can confirm that my example here is fully working now in IE11+ and I have merged the code into one routine as @Pullo did with his example.

The code from @pullo is obviously much neater and the way to go (too many 'ifs' in my example) but I'm happy that mine is at least working :slight_smile:

I do note that in the version from @pullo that you are continually appending the detect element to the dom in your check and you end up with loads of them if you open and close the window a lot/

e.g.

<detect></detect><detect></detect><detect></detect><detect></detect><detect></detect><detect></detect><detect></detect></body>
</html>

It would be better to set a once only flag as in my example and use that instead of manipulating the dom each time. Just check it once add a flag and then remove the detect element to clean up as in my latest example.

...and just for fun I converted my last example into jquery to see what would happen :slight_smile:


#51

Lol, oh yeah. Well spotted. This check should obviously be performed once and the stored in a variable.


#52

I know it was starting to get stressful, I'm glad you hung in there. I think it was a good exercise for us. With display:contents; coming soon in Chrome it will be good to know how to provide a polyfill for other browsers.

I noticed that too the other day, I wasn't sure if that was supposed to be happening or not.

Pullo, how would I go about making that modification in your code?


#53

Like this, good Sir:

<!doctype html>
<html lang="en">
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <link rel="stylesheet" href="http://www.rayswoodworks.com/rwd-test/js-test/css/screen.css" media="all">
  <link rel="stylesheet" href="http://www.rayswoodworks.com/rwd-test/js-test/css/nav-menu.css" media="all">
  <title>Slide Toggle</title>
  <style>
    h1 {font-size:1.6em}
    h2 {font-size:1.3em}
    p {font-size:1.1em;}
    code {font-size:1.1em; background:#fff}
  </style>
</head>
<body>

<div class="wrap">
  <header>
    <h1>Slide Toggle<br> only works in Firefox</h1>
  </header>
  <div class="inner">
    <div class="sidebar">
      <nav class="top">
        <button class="toggle"><span>Menu</span></button>
        <ul class="menu">
          <li><a href="#">Page Link 1</a></li>
          <li><a href="#">Page Link 2</a></li>
          <li><a href="#">Page Link 3</a></li>
          <li><a href="#">Page Link 4</a></li>
          <li><a href="#">Page Link 5</a></li>
          <li><a href="#">Page Link 6</a></li>
        </ul>
      </nav>
      <div class="mid">
        <p>Mid div space-around</p>
        <p>Mid div space-around</p>
        <p>Mid div space-around</p>
      </div>
      <div class="bot">
        <p>Side Bottom</p>
        <p>Side Bottom</p>
      </div>
    </div>
    <div class="main">
      <main>
        <h2>Same Page Layout</h2>

        <p>Same flexbox layout using <code>display:contents</code> to remove the <code>.sidebar</code> div and leave it's child nodes in place.
        This happens when the screen width is 650px and under. At this time only Firefox supports <code>display:contents</code>.
        Firefox actually leaves the <code>.sidebar</code> div in the html but it basically acts as if it's not there, and there's
        been no changes to the DOM. The child nodes now act as if they were children of the <code>.inner</code> div, which was
        previously their grandparent.</p>

        <p>That is why the Slide Toggle menu works properly in Firefox right now. Other browsers require a page reload when the media query kicks in.</p>

        <h2>The Same Workaround as other page</h2>

        <p>Browsers that don't support <code>display:contents</code> get the <code>unwrap.js</code> script. The script removes the
        <code>.sidebar</code> div and appends the child nodes back into the html. The nodes are now children of the <code>.inner</code> div.
        But unlike <code>display:contents</code> explained above, the script actually DOES remove the <code>.sidebar</code> div from the html. The child
        nodes have temporarily lost their place in the DOM unless the page gets reloaded. Once again, all this happens when screen width is 650px and under.</p>

        <h2>Now we have the "Slide Toggle" menu on this page</h2>

        <p>This is where I need help to target <code>&lt;button class="toggle"&gt;</code> from the <code>.inner</code> div to begin with.
        Then locate <code>&lt;ul class="menu"&gt;</code> by way of <code>.nextElementSibling</code>.</p>

        <p>I neeed help setting up a new ClickHandler function for the "Slide Toggle" script (view page source), that targets everything by way of the <code>.inner</code> div.

        Then when the <code>.sidebar</code> div is removed by browsers that don't support <code>display:contents</code>, the slide
        toggle should still work without needing a page reload when the browser window changes widths.</p>

        <p>Anyone willing to help with this can find my files here on the <a href="http://www.rayswoodworks.com/rwd-test/js-test/">Index Page</a>,
        as well as a <a href="http://www.rayswoodworks.com/rwd-test/js-test/">zip file</a> to pull them down in one shot. <br><br> Thanks for Reading!</p>

      </main>
    </div>
  </div>
  <footer>Footer</footer>
</div>

<script>
  (function(){
    "use strict";

    function unwrap(el){
      var docFrag = document.createDocumentFragment();
      while (el.firstChild) {
        var child = el.removeChild(el.firstChild);
        docFrag.appendChild(child);
      }

      el.parentNode.replaceChild(docFrag, el);
    }

    function supportsDisplayContents(){
      var newEl = document.createElement('detect');
      document.body.appendChild(newEl);
      return getComputedStyle(newEl).getPropertyValue('display') === 'none';
    }

    function toggleMenuClickHandler(evt) {
      var clickedEl = evt.target;
      if(clickedEl.className.match('toggle')) {
        var handler = clickedEl;
        var menuNode = clickedEl.nextElementSibling;
        var isExpanded = menuNode.classList.toggle('expand');
        menuNode.style.height = isExpanded ? originalHeight + 'px' : 0;
        handler.classList.toggle('open');
      }
    }

    function handleViewportChange(mql) {
      if (mql.matches) {
        console.log('Viewport smaller than 651px');
        if (noDisplayContents) sidebarParent.innerHTML = sidebarParentAltered;
        document.querySelector('.menu').style.height = 0;
      } else {
        console.log('Viewport larger than 651px');
        if (noDisplayContents) sidebarParent.innerHTML = sidebarParentNormal;
        document.querySelector('.menu').style.height = 'auto';
        document.querySelector('.menu').classList.remove('expand');
        document.querySelector('.toggle').classList.remove('open');
      }
    }

    var noDisplayContents = supportsDisplayContents();
    var inner = document.querySelector('.inner');
    var originalHeight = document.querySelector('.menu').offsetHeight;
    var mql = window.matchMedia('screen and (max-width: 650px)');

    inner.addEventListener('click', toggleMenuClickHandler);
    mql.addListener(handleViewportChange);

    if (noDisplayContents){
      var sidebar = document.querySelector('.sidebar');
      var sidebarParent = sidebar.parentNode;

      var sidebarParentNormal = sidebarParent.innerHTML;
      unwrap(sidebar);
      var sidebarParentAltered = sidebarParent.innerHTML;
    }

    handleViewportChange(mql);
  })();

</script>
</body>
</html>

#54

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