-1

Hey I'm trying to make a function that prints out the last character of the id of the element it's called from. This is simply a test before I move forward with my actual plans but I'm stuck here. The Issue I'm getting is that I get an error saying that my parameter is 'undefined', so there is not value being passed, or the value being passed isn't a string and thus string operations (charAt) won't work on it. I have included code. Thank you.

My webpage have an undertermined amount of modal images which , when clicked, open a slideshow album of images. I got this working for 1 image then realized that to have it work for an undetermined amount of slideshows of undetermined size I should make a function that fills the slideshow div. I planned to have each modal image to have an id of "modal-1, modal-2...etc" and have a bunch of arrays with the images each named similarly "slides_1 ,slides_2...etc" and if the last character of both match up, then it will populate the slideshow with the images in the array. This way If i need to add another modal image all i need to do is give it the appropriate id and add an array of its images.this is a ,past question which sort of illustrates that larger goal.

html:

<body id="modal-2" onload="fillSlides(this.id)">

   <h2 id="title" style="text-align:center"></h2>

  <div class="row">
    <div class="column">
      <img id="modal-1" src="https://www.yosemitehikes.com/images/wallpaper/yosemitehikes.com-bridalveil-winter-1200x800.jpg" style="max-width:100%" onclick="openModal();currentSlide(1);fillSlides(this.id);" class="hover-shadow cursor">
    </div>
  </div>

Javascript:

function fillSlides(modalID){
      var slides_1 = ["Images/LS_01.jpg", "Images/LS_02.jpg", "Images/LS_03.jpg", "Images/LS_04.jpg" ]
      var modalI = modalID;
      var lastCharM = modalI.charAt(modalI.length - 1);

       document.getElementById("title").innerHTML = "the last char is" + lastCharM;


    }
BrownBoii333
  • 159
  • 1
  • 2
  • 12
  • You haven't shared what you end-goal is. It's entirely possible there's a better approach, as relying on parsed strings is often not the best / most reliable approach. For example, have you considered using a data attribute ? – devlin carnate Nov 07 '17 at 19:22
  • I don't know what a data attribute is, sorry I'm new to js. I will add an edit which says my end goal – BrownBoii333 Nov 07 '17 at 19:24
  • A [data attribute](https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes) is something that you add to the HTML element, and it can be used to hold information you need. So, for example, if you're trying to determine which element was clicked, you could store a number in the data attribute rather than append a number to the end of an id. – devlin carnate Nov 07 '17 at 19:27
  • hm okay thanks ya I suppose that would work better. I'll try that. Also I have added more explanation to my post – BrownBoii333 Nov 07 '17 at 19:31
  • How would you use a data-attribute? Like if I wanted to print a data-attribute like i'm at the moment trying to print the id's last character how would I do this? – BrownBoii333 Nov 07 '17 at 19:40
  • Here's a simple example: https://jsfiddle.net/zephyr_hex/813aeq1w/ – devlin carnate Nov 07 '17 at 19:47
  • 1
    @devlincarnate Instead of using `document.getElementsByClassName("span");` I think is best to use `document.querySelectorAll('span[data-num]');`. This way you attach the event only to the span that has the attribute data-num. – kakamg0 Nov 07 '17 at 19:50
  • I've tried something similar here, why won't it work? @devlincarnate https://jsfiddle.net/pcghdwd6/ – BrownBoii333 Nov 07 '17 at 19:52
  • See here: https://stackoverflow.com/questions/5468350/javascript-not-running-on-jsfiddle-net I've updated your Fiddle so it works: https://jsfiddle.net/zephyr_hex/pcghdwd6/1/ – devlin carnate Nov 07 '17 at 20:05
  • @kakamg0 : yes, that's a better approach. – devlin carnate Nov 07 '17 at 20:20

2 Answers2

4

The load event is actually a window event, not actually an event on body, so this within your onload-attribute-style handler text is window. To access body, use document.body instead of this:

<body id="modal-2" onload="fillSlides(document.body.id)">

Live example on JSBin (since Stack Snippets want to supply the body markup for you).


In general, I'd recommend avoiding onxyz-attribute-style event handlers. For one thing, other than methods on the element or (sometimes) it's containing element, the only functions they can call have to be globals, and the global namespace on browsers is already really crowded.

Instead, I'd recommend at least using the onxyz properties on elements and on window. In this case, that would look like this in your JavaScript code:

window.onload = function() { loadSlides(document.body.id); };

But even better would be to use modern (DOM2+) event handling via addEventListener (with fallback to Microsoft's proprietary attachEvent if you need to support obsolete versions of IE; details here):

window.addEventListener("load", function() {
    loadSlides(document.body.id);
};
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • I simply put it in the onload to test it but really I want the function to load when it the image is clicked (see edit), sorry I didn't explain better. How do I do that because it isn't working – BrownBoii333 Nov 07 '17 at 19:41
1

I would approach this using data attributes instead of parsing a string. This will allow you to use a class instead of id's on your HTML img elements, and then determine which element was clicked on via the data attribute.

HTML

<img data-num="1" src="https://www.yosemitehikes.com/images/wallpaper/yosemitehikes.com-bridalveil-winter-1200x800.jpg" style="max-width:100%" class="hover-shadow cursor img">

JS

var classname = document.getElementsByClassName("img");

var myFunction = function() {
    var attribute = this.dataset.num;
    //attribute has the data-num value
};

for (var i = 0; i < classname.length; i++) {
    classname[i].addEventListener('click', myFunction, false);
}

I'm also binding the click event in JS rather than an inline onclick in order to preserve separation of concerns (not mixing JS and HTML).

Update, in response to comment by kakamg0:

Using document.querySelectorAll('img[data-num]') would omit the need to use a class for targeting.

HTML

<img data-num="1" src="https://www.yosemitehikes.com/images/wallpaper/yosemitehikes.com-bridalveil-winter-1200x800.jpg" style="max-width:100%" class="hover-shadow cursor">

JS

var images = document.querySelectorAll('img[data-num]');

var myFunction = function() {
    var attribute = this.dataset.num;
    //attribute has the data-num value
};

for (var i = 0; i < images.length; i++) {
    images[i].addEventListener('click', myFunction, false);
}
devlin carnate
  • 8,309
  • 7
  • 48
  • 82
  • This doesn't seem to be working, what have I done wrong? https://jsfiddle.net/pcghdwd6/ – BrownBoii333 Nov 07 '17 at 20:08
  • @BrownBoii333 : See [here](https://stackoverflow.com/questions/5468350/javascript-not-running-on-jsfiddle-net) for an explanation. Here's a working version: https://jsfiddle.net/zephyr_hex/pcghdwd6/1/ – devlin carnate Nov 07 '17 at 20:16