CSS attribute not always being grabbed by javascript correctly

Viewed 49

I am trying to resize the side bars whenever the image changes.

I have my javascript trying grab the height of the image after it changes

var imgHeight = $('#mainImg').height();
var currImg = 0;

var imagesSet = ["1.jpg", "2.jpg", "3.jpg", "4.jpg", "5.jpg", "6.jpg"];
var imageLoc = "images/zalman/"

$('#bttnRight').click(function(){
  nextImg();
  imgHeight = $('#mainImg').height();
  resizeBttn();
});

function nextImg(){
  currImg++;
  if(currImg>=imagesSet.length){
      currImg=0;
  }
  $('#mainImg').attr("src",imageLoc + imagesSet[currImg]);
}

function resizeBttn() {
  $(document).ready(function() {
        $('#bttnLeft').css("height",imgHeight);
        $('#bttnLeft').css("bottom",imgHeight/2-5);
  });
  $(document).ready(function() {
        $('#bttnRight').css("height",imgHeight);
        $('#bttnRight').css("bottom",imgHeight/2-5);
  });
}

for some reason, it doesn't always grab the height at the correct time and the side bars will stay at the previous height.

Below I have a JSFiddle that should be working the way my setup is.

Please excuse any inconsistencies and inefficiencies, I am learning.

Just seems weird that it would sometimes grab the height and sometimes not.

I will also be attaching an image of what I see sometimes from the JSfiddle.

I will also attach an image of what I see on my site I am actually writing.

enter image description here

enter image description here

https://jsfiddle.net/6bewkuo5/6/

2 Answers

The issue is because your JavaScript accessing the height of the image before the image as actually been re-rendered in the DOM. Adding a slight delay after assigning the new image source may help things, but...

You actually don't need to use JavaScript to set the height of the buttons

You can achieve what you're after by placing the buttons and image inside of a container with css attribute display: flex.

Like this:

.container {
  display: flex;
  justify-content: center;
}
<div class="container">
  <button class="prev">&lt;</button>
  <img src="https://www.avalonwinery.com/wp-content/uploads/2013/12/200x300.gif">
  <button class="next">&gt;</button>
</div>

Elements within a flex container will automatically fill the height, this includes buttons. Because the images will automatically adjust the height of the container, the buttons will also automatically adjust their height to match.

Run the example below

const images = [
  "https://www.avalonwinery.com/wp-content/uploads/2013/12/200x300.gif",
  "https://images.sftcdn.net/images/t_app-logo-xl,f_auto/p/ce2ece60-9b32-11e6-95ab-00163ed833e7/1578981868/the-test-fun-for-friends-logo.png",
  "https://hiveconnect.co.za/wp-content/uploads/2018/05/800x600.png",
  "https://upload.wikimedia.org/wikipedia/commons/b/b5/800x600_Wallpaper_Blue_Sky.png"
]

const imageEl = document.querySelector('img')
let imageIndex = 0

document.querySelector('.prev').addEventListener('click', e => {
  if (--imageIndex < 0) { imageIndex = images.length - 1 }
  imageEl.src = images[imageIndex]
})

document.querySelector('.next').addEventListener('click', e => {
  if (++imageIndex > images.length - 1) { imageIndex = 0 }
  imageEl.src = images[imageIndex]
})
body {
  background-color: #206a5d;
  color: #ffffff;
  text-align: center;
}

.container {
  display: flex;
  justify-content: center;
}

img {
  max-width: 50%;
}
<h1>Zalman Build</h1>
<div class="container">
  <button class="prev">&lt;</button>
  <img src="https://www.avalonwinery.com/wp-content/uploads/2013/12/200x300.gif">
  <button class="next">&gt;</button>
</div>

The reason is because the resizeBttn code is firing before the image has actually finished downloading and loading into the DOM. I made these changes in your fiddle:

var imgHeight = $('#mainImg').height();
var currImg = 0;

var imagesSet = ["https://www.avalonwinery.com/wp-content/uploads/2013/12/200x300.gif","https://images.sftcdn.net/images/t_app-logo-xl,f_auto/p/ce2ece60-9b32-11e6-95ab-00163ed833e7/1578981868/the-test-fun-for-friends-logo.png", "https://hiveconnect.co.za/wp-content/uploads/2018/05/800x600.png", "https://upload.wikimedia.org/wikipedia/commons/b/b5/800x600_Wallpaper_Blue_Sky.png"];
var imageLoc = "images/zalman/"


$(document).ready(function() {
  resizeBttn();
});

$( window ).resize(function() {
  /* imgHeight = $('#mainImg').height() */; // commented out; we do this in resizeBttn now
  resizeBttn();
});

$('#bttnLeft').click(function(){
  prevImg();
  /* imgHeight = $('#mainImg').height() */; // commented out; we do this in resizeBttn now
  /* resizeBttn() */; // we do this as an `onload` to the image now
});

$('#bttnRight').click(function(){
  nextImg();
  /* imgHeight = $('#mainImg').height() */; // commented out; we do this in resizeBttn now
  /* resizeBttn() */; // we do this as an `onload` to the image now
});
function nextImg(){
  currImg++;
  if(currImg>=imagesSet.length){
      currImg=0;
  }
  $('#mainImg').attr("src",imagesSet[currImg]);
}

function prevImg(){
  currImg--;
  if(currImg<0){
      currImg=imagesSet.length-1;
  }
  $('#mainImg').attr("src",imagesSet[currImg]);
}

function resizeBttn() {
  imgHeight = $('#mainImg').height()
  // removed superfluous doc.ready
  $('#bttnLeft').css("height",imgHeight);
  $('#bttnLeft').css("bottom",imgHeight/2-5);
  $('#bttnRight').css("height",imgHeight);
  $('#bttnRight').css("bottom",imgHeight/2-5);
}

And then rewrote your <img /> tag to call resizeBttn on onload:

<img id="mainImg" src="https://www.avalonwinery.com/wp-content/uploads/2013/12/200x300.gif" onload="resizeBttn()"/>

You can see this in action in this fiddle.

Also, a few additional notes on your code, at a glance:

  • You have some invalid HTML; you're going to want to run that through an HTML validator and fix it, because sometimes it is fine, but sometimes it can lead to all sorts of strange behavior.
  • You're playing fast and l0ose with global variables in your JS that get set in different functions; it might work OK while the script is small, but as things scale it can quickly become difficult to maintain
  • You should really avoid abusing the onclick to get link-like behavior from <li> elements; it can impact SEO as well as accessibility. I'd recommend simply using an anchor element inside or outside the <li>
  • I'd recommend taking a close look at this answer by user camaulay; he makes an excellent point that this may not require JS at all- if a more elegant solution exists w/ CSS it is probably going to be more performant and maintainable.
Related