admin管理员组

文章数量:1389903

Background: I'm working on a page for an artist and I've run into some interesting behavior with the site.

I'm using flexbox rather than bootstrap for the layout because the site is pretty small and doesn't have a ton of elements. Flexbox has been great for laying out the site especially with respect to changes for different media sizes.

In the smallest media size, however, the mobile layout, I'm having some trouble. I like the look of the collapsible menu from bootstrap but because I'm not using bootstrap, I got the icon from font awesome and now I'm trying to work in the collapsible menu properties manually using JavaScript and CSS. But, without using bootstrap, I'm not sure why my expandable menu only works after repeated clicks, not on the first click.

Expectation: In the smallest media query (window <768 pixels), i have the default mobile layout set up the way I want it. The banner sits about half way down the page, and the hamburger menu is fixed to the top of the browser. When I click the menu button the reaction I'm expecting is to have the menu links appear below the menu button and for the page banner to disappear.

Problem: This happens and toggles but only after clicking multiple times.

questions: 1. what is causing the clickable behavior to only function after a few clicks? 2. why won't the display property of 'none' apply to the 'nav-left' item? 3. when the menu displays, other than using margin to offset the menu items that appear, is there a way to ensure that they appear below the hamburger button area? (i.e. i want links 1-3 to show up below the hamburger menu when they appear).

jsfiddle here

function menuToggle() {
  var x = document.getElementById('nav-right');
  var y = document.getElementsByClassName("nav-left");
  if (x.style.display == "none") {
    x.style.display = "flex";
    y.style.display = "none";
  } else {
    x.style.display = "none";
    y.style.display = "flex";
  }
}
* {
  margin: 0;
  padding: 0;
}

html,
body {
  background-color: gray;
  height: 100%;
  width: 100%;
}

.nav-left {
  font-family: 'Kristi', cursive;
  font-size: 5.5em;
  color: #e319b6;
  letter-spacing: 2px;
  margin: 0;
  padding: 0 0 0 50px;
}

.nav-left a {
  text-decoration: none;
  color: #e319b6;
}

.hamburger {
  display: none;
}

li a {
  text-decoration: none;
  color: white;
}

li a:hover {
  text-decoration: none;
  color: #e319b6;
}

li {
  padding: 50px 10px;
  list-style: none;
  font-family: 'Montserrat', sans-serif;
  font-size: 1.7em;
  font-weight: 100;
  color: white;
  display: inline-block;
}

@media (max-width:767px) {
  .hamburger {
    width: 100%;
    padding: 2% 0;
    display: flex;
    color: white;
    font-size: 2em;
    justify-content: center;
    background: rgba(0, 0, 0, .1);
    position: absolute;
    cursor: pointer;
    border: none;
  }
  .pagenav {
    width: 100%;
    display: flex;
    align-items: center;
    flex-direction: column;
  }
  .nav-left {
    width: 100%;
    display: flex;
    justify-content: center;
    order: 2;
    margin-top: 10%;
    padding: 0;
  }
  #nav-right {
    display: none;
    justify-content: center;
    width: 100%;
  }
  .nav {
    width: 100%;
  }
  nav ul {
    display: flex;
    flex-direction: column;
    width: 100%;
  }
  nav ul li {
    display: flex;
    justify-content: center;
    width: 100%;
  }
  nav ul li {}
  nav ul li:hover {
    background: rgba(0, 0, 0, .8);
  }
  nav ul li:hover a {
    color: #e319b6;
  }
}
<head>
  <meta charset="utf-8">
  <title></title>


  <!-- Font Awesome -->
  <link rel="stylesheet" href=".1.0/css/all.css" integrity="sha384-lKuwvrZot6UHsBSfcMvOkWwlCMgc0TaWr+30HWe3a4ltaBwTZhyTEggF5tJv8tbt" crossorigin="anonymous">


</head>

<body>

  <div class="pagenav">

    <div class="nav-left">
      placeholder
    </div>

    <button class="hamburger" onclick="menuToggle()">
                  <i class="fas fa-bars"></i>
              </button>

    <nav id="nav-right">
      <ul>
        <li><a href="reel.html">Link 1</a></li>
        <li><a href="">Link 2</a></li>
        <li><a href="">Link 3</a></li>
      </ul>

    </nav>

  </div>


</body>

Background: I'm working on a page for an artist and I've run into some interesting behavior with the site.

I'm using flexbox rather than bootstrap for the layout because the site is pretty small and doesn't have a ton of elements. Flexbox has been great for laying out the site especially with respect to changes for different media sizes.

In the smallest media size, however, the mobile layout, I'm having some trouble. I like the look of the collapsible menu from bootstrap but because I'm not using bootstrap, I got the icon from font awesome and now I'm trying to work in the collapsible menu properties manually using JavaScript and CSS. But, without using bootstrap, I'm not sure why my expandable menu only works after repeated clicks, not on the first click.

Expectation: In the smallest media query (window <768 pixels), i have the default mobile layout set up the way I want it. The banner sits about half way down the page, and the hamburger menu is fixed to the top of the browser. When I click the menu button the reaction I'm expecting is to have the menu links appear below the menu button and for the page banner to disappear.

Problem: This happens and toggles but only after clicking multiple times.

questions: 1. what is causing the clickable behavior to only function after a few clicks? 2. why won't the display property of 'none' apply to the 'nav-left' item? 3. when the menu displays, other than using margin to offset the menu items that appear, is there a way to ensure that they appear below the hamburger button area? (i.e. i want links 1-3 to show up below the hamburger menu when they appear).

jsfiddle here

function menuToggle() {
  var x = document.getElementById('nav-right');
  var y = document.getElementsByClassName("nav-left");
  if (x.style.display == "none") {
    x.style.display = "flex";
    y.style.display = "none";
  } else {
    x.style.display = "none";
    y.style.display = "flex";
  }
}
* {
  margin: 0;
  padding: 0;
}

html,
body {
  background-color: gray;
  height: 100%;
  width: 100%;
}

.nav-left {
  font-family: 'Kristi', cursive;
  font-size: 5.5em;
  color: #e319b6;
  letter-spacing: 2px;
  margin: 0;
  padding: 0 0 0 50px;
}

.nav-left a {
  text-decoration: none;
  color: #e319b6;
}

.hamburger {
  display: none;
}

li a {
  text-decoration: none;
  color: white;
}

li a:hover {
  text-decoration: none;
  color: #e319b6;
}

li {
  padding: 50px 10px;
  list-style: none;
  font-family: 'Montserrat', sans-serif;
  font-size: 1.7em;
  font-weight: 100;
  color: white;
  display: inline-block;
}

@media (max-width:767px) {
  .hamburger {
    width: 100%;
    padding: 2% 0;
    display: flex;
    color: white;
    font-size: 2em;
    justify-content: center;
    background: rgba(0, 0, 0, .1);
    position: absolute;
    cursor: pointer;
    border: none;
  }
  .pagenav {
    width: 100%;
    display: flex;
    align-items: center;
    flex-direction: column;
  }
  .nav-left {
    width: 100%;
    display: flex;
    justify-content: center;
    order: 2;
    margin-top: 10%;
    padding: 0;
  }
  #nav-right {
    display: none;
    justify-content: center;
    width: 100%;
  }
  .nav {
    width: 100%;
  }
  nav ul {
    display: flex;
    flex-direction: column;
    width: 100%;
  }
  nav ul li {
    display: flex;
    justify-content: center;
    width: 100%;
  }
  nav ul li {}
  nav ul li:hover {
    background: rgba(0, 0, 0, .8);
  }
  nav ul li:hover a {
    color: #e319b6;
  }
}
<head>
  <meta charset="utf-8">
  <title></title>


  <!-- Font Awesome -->
  <link rel="stylesheet" href="https://use.fontawesome./releases/v5.1.0/css/all.css" integrity="sha384-lKuwvrZot6UHsBSfcMvOkWwlCMgc0TaWr+30HWe3a4ltaBwTZhyTEggF5tJv8tbt" crossorigin="anonymous">


</head>

<body>

  <div class="pagenav">

    <div class="nav-left">
      placeholder
    </div>

    <button class="hamburger" onclick="menuToggle()">
                  <i class="fas fa-bars"></i>
              </button>

    <nav id="nav-right">
      <ul>
        <li><a href="reel.html">Link 1</a></li>
        <li><a href="">Link 2</a></li>
        <li><a href="">Link 3</a></li>
      </ul>

    </nav>

  </div>


</body>

  • javascript
  • css
  • flexbox
Share Improve this question edited Jul 8, 2018 at 4:27 Panayiotis Spanos asked Jul 8, 2018 at 4:15 Panayiotis SpanosPanayiotis Spanos 1551 gold badge3 silver badges17 bronze badges 9
  • Your script has an error: y gets an HTMLcollection, not an element. You probably want y[0] – flen Commented Jul 8, 2018 at 4:31
  • would you suggest i instead give the div an 'id' for targeting and replace the y target with a getElementById for that target 'id'? For whatever reason, it seems to work even targeting the class collection i've specified but just not on the first click. – Panayiotis Spanos Commented Jul 8, 2018 at 4:33
  • The reason it's not working on the first click was explained by chbchb55, since in the first pass of the function it reads x.style.display and this equals "", and not "none". If it did equal "none", then it would work in the first pass – flen Commented Jul 8, 2018 at 4:37
  • 1 Then just grab the reference by using first index var y = document.getElementsByClassName("nav-left")[0]; @PanayiotisSpanos – Debojyoti Commented Jul 8, 2018 at 4:46
  • 1 @PanayiotisSpanos about it being assigned "none": actually no, but your logic is good. The thing is the DOM element attribute JavaScript is modifying in style is not set to anything by default. Your logic is: well, it should read the CSS values or whatever values the element display has. But it doesn't, by default it has none (""), only when you change it via JavaScript it has anything – flen Commented Jul 8, 2018 at 4:57
 |  Show 4 more ments

2 Answers 2

Reset to default 4

The issue is in the HTML, not your function

The problem you're encountering is that if (x.style.display == "none") should pass the first time around, because it's hidden right? Well, it isn't passing because, according to the style attribute, display does not equal none, therefore, you can't tell that it's hidden. To fix this just do:

<nav id='nav-right' style='display: none'>

Now you're code knows that #nav-right is hidden and will work on the first click.

Option 2, Modify the code:

You can use a one-time toggle variable. With this, the first time the button is clicked it will show the menu and then set the toggle variable to false. Like so:

let firstMenuClickToggle = true

function menuToggle() {
  ...
  if (x.style.display == 'none' || firstMenuClickToggle) {
    firstMenuToggle = false
    ...

Option 3, check for empty string and 'none' at the same time

As @flen has said, you can search for an empty string alongside 'none', but why not do both at the same time.

For example, try replacing your if statement with the following:

if (!'none'.indexOf(x.style.display)) {

This is actually, from my tests with Benchmark.js, significantly faster than @flen's separate parisons, almost a Five Second difference when each is run 10,000 times.

This works because both of the strings '' and 'none' exist in 'none' technically, therefore, if either of these values are there, they will return 0. As such, I have used the ! operator to check for this, !0 will always return true but any other number (i.e. !-1) will return false.

Edit: Making it more readable

String.prototype.emptyOrEqualTo = function(string) {
  return string.indexOf(this)
}

// in your function
if (x.style.display.emptyOrEqualTo('none')) {

While modifying String.prototype is not usually done it does produce a higher readability than most other methods of increasing readability. In fact, you could even throw an is onto that and it would be a sentence.

display.isEmptyOrEqualTo('none')

Now you can actually read it:

If display is empty or equal to "none"

FYI, in your JSFiddle, y.style is undefined because you used .getElementsByClassName but that returns an array and you didn't get the first index of the returned array.

Another few notes:

  • I would suggest using more descriptive variable names than x and y as they do not municate their meaning, you have to lookup their definition line to understand, which is not exactly a "best practice".
  • You should probably move your javascript to a separate file like you've already done with your CSS.
  • Optional: I would suggest learning to use const and let as well.
  • Keep on keeping on, you're doing great.

chbchb55 already explained what's wrong, but I don't think you should change your HTML to fix a bug in the JavaScript code. Rather, change your menuToggle function to this here:

function menuToggle() {
  var x = document.getElementById('nav-right');
  var y = document.getElementsByClassName("nav-left")[0]; //changed to get the first element of the HTMLCollection
  if (x.style.display == "none" || x.style.display === "") { //changed to also accept the original value, which is "" instead of "none"
    x.style.display = "flex";
    y.style.display = "none";
  } else {
    x.style.display = "none";
    y.style.display = "flex";
  }
}

Just to be clear, let me rephrase the sollution: the first time you call your menuToggle function, it reads x.style.display and its value is actually "" and not "none" as you seemed to be expecting. Since it is not "none", it runs the else clause and goes on to assign x.style.display to "none" and y.style.display to "flex". This shouldn't happen, because this signifies that your menu is closed. So only on the second call it opens it, because the first call of the function ran the else clause and closed the (already closed) menu.

Adendum:
chbchb55 also suggested an ingenious optimization, using if (!'none'.indexOf(x.style.display)) or even creating a prototype as syntactic sugar to make this more readable. I'd still argue against using this for 2 reasons.

First, it is still less readable than if (x.style.display == "none" || x.style.display === ""). Here whoever reads the code can immediately see that we are checking for 2 conditions, and only these 2. Using indexOf, it's not immediately clear that we are also checking for "", so we'd have to specify this in a ment. It's worse for readability, in my opinion.

Second, if speed is an issue, this here is actually the fastest sollution, substituting the if for the else clause: if (x.style.display === "flex"), then do what's now in the else clause. This way, else will catch x === '' and x == 'none' and whatever else. If length of the code is an issue, we could improve this further using the ternary operator:

function menuToggle() {
  var x = document.getElementById('nav-right');
  var y = document.getElementsByClassName("nav-left")[0]; //changed to get the first element of the HTMLCollection
  x.style.display === "flex" ? (x.style.display = "none", y.style.display = "flex") : (x.style.display = "flex", y.style.display = "none");     
}

Still, speed and length are not an issue here, as the code is now. If it improved 1ms I'd be impressed. I think the original solution is the most readable one, that's why I'd still suggest it as answer. Nevertheless, thinking of using indexOf was pretty clever, and if I'm not mistaken it's faster than ES6 includes

本文标签： javascript and css button doesn39t work on first click amp FlexboxStack Overflow

版权声明：本文标题：javascript and css button doesn&#39;t work on first click &amp; Flexbox - Stack Overflow 内容由网友自发贡献，该文观点仅代表作者本人， 转载请联系作者并注明出处：http://www.betaflare.com/web/1744612942a2615764.html， 本站仅提供信息存储空间服务，不拥有所有权，不承担相关法律责任。如发现本站有涉嫌抄袭侵权/违法违规的内容，一经查实，本站将立刻删除。