"Image gallery" assessment


(Chris Mills) #4

I was hoping not to self mark really this would help me get a proper unbiased assessment grade. I’m sure you understand when your working on your own as I have been it’s easy to con yourself that your work meets the grade…

I agree with you. However, unfortunately we haven’t got the resources to manually assess everyone’s entries. I wish we did.

However I have to say on this occasion I feel as though I’m pretty close the only real difference is that I have used an array to hold all the photo path values. But to me this is better than making up the path in the loop because you don’t have to change the code to change the photo - just substitute or add to the array, I have set the loop using the array.length property anyway. I think the rest is the same looking at what is required in the marking guide. What do you think.* *

Yeah, cool, that all sounds fine. I agree that the array is a better way of doing this. In a real app, you’d probably retrieve the photos and info via some kind of JSON structure via an XHR call, or similar.


(Juan de Souza) #5

Hi everyone! I’m stuck in the Image Gallery assessment. Can anyone show me the code? It’s really annoying that they do not show the code on the page!


(Chris Mills) #6

You can find the finished code here:


(Paul Harris) #7

I have a question about the adding an onclick handler in https://developer.mozilla.org/en-US/docs/Learn/JavaScript/Building_blocks/Image_gallery section

https://pastebin.mozilla.org/9069371 this is what I have so far

I’m confused as to how I get the “src” without using newImage as the event object won’t contain the information either if i set the event object to bbb it’s still the same thing as using newImage


(adilson) #8

@Paul_Harris Unfortunately your code is no longer available, so I can’t see what you had made. In the guidelines, think Chris gave us a hint with “event” and “target” and suggested us to use Event objects as is explained in the course, or at least that’s what I used (haven’t checked the solved code).


(adilson) #9

And this is my JS code and live preview . Only points to highlight in my code comparing to the solved one by chris are:

  1. I used addEventListener instead of using the event handler as property. Being able to remove the listeners was what made me decide for it (even if it didn’t matter at this early stage :sweat_smile:).
  2. Because of the previous point, I didn’t code the displayImage function specified in the guidelines. It was going to be only one line in there, and I was already using event function handlers. Too many functions for something that simple. But, I was able to see that setting up displayImage allow us to used it in a wide different set of events in case we want to improve our gallery; i.e. dragging a file to the page, display such picture and show another thumbnail at the bottom.

I loved this assessment! it was challenging. I’m going to read again the “Introduction to events” section as well as the external links in there. I felt like it was a stepped curve of learning.


(Patty518) #10

Hello!

my code is the following:

for( i=1; i<=5; i++) {
var newImage = document.createElement(‘img’);
newImage.setAttribute(‘src’, ‘images/pic’ + i + ‘.jpg’);

function whatever(e) {
displayedImage.setAttribute(‘src’, e.target.getAttribute(‘src’));
}

newImage.addEventListener(‘click’, whatever);
thumbBar.appendChild(newImage);
}

It seems to be working. My questions are the following:

  1. is there a preference for having the function ‘whatever’ inside the for loop or outside the for loop? mine is within the loop, but the solution code places it outside. Just want to know if there are any cases, and if so, what they are, in which one would be preferable.

  2. does the placement for the line ‘thumbBar.appendChild(newImage);’ matter, or can it be placed anywhere inside the for loop?

  3. I see that in the solution code, the function displayImage(imgSrc) is called before the function is defined, but I was under the impression that the code would crash if a function is called before it’s defined. can you please clarify the order of function calling and definition?

And finally, just want to say that i found this assessment to be challenging and fun!


(Chris Mills) #11

Hi there Patty,

Thanks for sharing your solution. I had a bit of a play, and ended up with this:

/* Looping through images */

function displayImage(e) {
  displayedImage.setAttribute('src', e.target.getAttribute('src'));
}

for(var i = 1; i <= 5; i++) {
  var newImage = document.createElement('img');
  newImage.setAttribute('src', 'images/pic' + i + '.jpg');
  thumbBar.appendChild(newImage);
  newImage.addEventListener('click', displayImage);
}

I think that if anything this is a better solution than our final example, although obviously I’ve got the function outside the solution. But you’ve improved on it by adding an event listener, rather than using an onclick handler function. Again, removing a function from the loop.

It is generally considered bad practice to include a function inside a loop, as it can cause problems in some circumstances. It is better to define the function outside the loop and reference it from inside.

I am going to update our example when I get the time to use something more like this. I’ve made a note of it. Thanks again for jogging me on this!

To answer your questions:

  1. See above.
  2. There is flexibility here, but it does need to be placed after you have defined newImage, otherwise you’ll get an undefined error.
  3. the answer here is “it depends” — this stack overflow post provides useful reading on bthe matter: https://stackoverflow.com/questions/9973461/does-a-javascript-function-have-to-be-defined-before-calling-it

(Mattia Lorenz) #12

Hi guys,

this is my code and it’s currently working. What do you think?

/* Looping through images */
for (i = 1; i <= 5; i++) {
var newImage = document.createElement(‘img’);
newImage.setAttribute(‘src’, ‘./images/pic’ + i + ‘.jpg’);
thumbBar.appendChild(newImage);
newImage.onclick = getTheImage;
function getTheImage(e, src) {
var src = e.target.getAttribute(‘src’);
displayedImage.setAttribute(‘src’, src);
};
};

/* Wiring up the Darken/Lighten button */
btn.onclick = changeOverlay;
function changeOverlay () {
var cl = btn.getAttribute(‘class’);
if (cl === ‘dark’) {
btn.setAttribute(‘class’, ‘light’);
btn.textContent = ‘Lighten’;
overlay.style.backgroundColor = ‘rgba(0,0,0,0.5)’;
}
else {
btn.setAttribute(‘class’, ‘dark’);
btn.textContent = ‘Darken’;
overlay.style.backgroundColor = ‘rgba(0,0,0,0)’;
}
};


(Chris Mills) #13

Hi Mattia,

Your code doesn’t look too at at all — nice job!

One bit of feedback I can give you — in the following block:

function getTheImage(e, src) {
var src = e.target.getAttribute('src');
displayedImage.setAttribute('src', src);
}
}

You don’t need to define the src parameter in the function, as you are getting src inside the function body in the second line instead. So the following would work fine:

function getTheImage(e) {
var src = e.target.getAttribute('src');
displayedImage.setAttribute('src', src);
}
}

#14
Problem

Hello, I have a problem here. I don’t understand what didn’t work in my function that tries to change the source of displayedImage. My code:

var displayedImage = document.querySelector('.displayed-img');
var thumbBar = document.querySelector('.thumb-bar');

btn = document.querySelector('button');
var overlay = document.querySelector('.overlay');

for (var i = 0; i < 5; i++) {
  var newImage = document.createElement('img');
  var path = 'images/pic' + (i + 1).toString() + '.jpg';
  newImage.setAttribute('src', path);
  thumbBar.appendChild(newImage);
}

var image = document.getElementsByTagName('img');

function sourceAlter(e) {
	var source = e.target.getAttribute('src');
	displayedImage.setAttribute('src', source);
}

image.onclick = sourceAlter;

Furthermore, when I tried to change image.onclick = sourceAlter; to image.addEventListener('click', sourceAlter), the console says that image.addEventListener is not a function. Why is that so?

EDIT:
I’ve found out the problem to my code. Apparently, getElementsByTagName returns and array and therefore, I have to access the array items one by one. This also caused problems when I tried to pass image directly to addEventListener. I’ve fixed my code to the following, please grade me :slight_smile: :

var displayedImage = document.querySelector('.displayed-img');
var thumbBar = document.querySelector('.thumb-bar');

btn = document.querySelector('button');
var overlay = document.querySelector('.overlay');

for (var i = 0; i < 5; i++) {
  var newImage = document.createElement('img');
  var path = 'images/pic' + (i + 1).toString() + '.jpg';
  newImage.setAttribute('src', path);
  thumbBar.appendChild(newImage);
}

var image = document.getElementsByTagName('img');

function sourceAlter(e) {
	var source = e.target.getAttribute('src');
	displayedImage.setAttribute('src', source);
}

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

/* Wiring up the Darken/Lighten button */
btn.onclick = function() {
	if (btn.getAttribute('class') === 'dark') {
		btn.setAttribute('class', 'light');
		btn.textContent = 'Lighten';
		overlay.style.backgroundColor = 'rgba(0, 0, 0, 0.5)';
	}

	else {
		btn.setAttribute('class', 'dark');
		btn.textContent = 'Darken';
		overlay.style.backgroundColor = 'rgba(0, 0, 0, 0)';
	}
}

(Chris Mills) #15

Hi @AvidLearner, congratulations on sorting your problem out! Pretty much all of the ways of return multiple element references will return an array that you need to loop through (whether it is getElementsByTagName, getElementsByName, or the more modern querySelectorAll).

I don’t tend to give you an exact grade, as I just don’t have time to mark all the assessment entries in detail. What I can do is test your code (it is a little different to mine, but it works fine, well done), and give you links to the marking guide and finished code so you can check it yourself:


#16

@chrismills I have some questions, though, if you don’t mind me asking.

  1. Here, my function sourceAlter only has the parameter e and therefore, I did not pass any parameters when I called the function, hence image[i].addEventListener('click', sourceAlter) without any parentheses and parameters. However, what if my function requires two parameters, e.g. sourceAlter(a, b), how do I pass on the parameters to said function?

  2. I don’t quite understand how looping works in JavaScript. It somehow does not make sense to me how for (...) in my code above worked. I do know that it loops five times (as expressed in the conditional statement i < image.length) and that whenever an image with the order i is clicked, we run a function. But, when we click picture 1, 2,3 and then 1 again, it somehow worked? There is not decrement in the for loop, so how did it happen? Is the loop run over again after each action (in this case, click (but doesn’t have to be necessarily a click))?


(Chris Mills) #17

So to answer the first question, this isn’t the simplest, but it is possible. This stack overflow answer is useful, and contains multiple ways of doing it: https://stackoverflow.com/questions/256754/how-to-pass-arguments-to-addeventlistener-listener-function

For the second question, the loop only runs once. But one of the main things the loop does is to add an onclick handler to each image, using this code:

newImage.onclick = function(e) {
    var imgSrc = e.target.getAttribute('src');
    displayImage(imgSrc);
}

because that handler is added to each image, it is run every time an image is clicked.


(Impresently) #18

I’m a little confused on how event objects work.

In my for loop the following code works:

for (i = 1; i <= 5; i++) {
    var newImage = document.createElement('img');
    newImage.setAttribute('src', "images\/pic" + i + ".jpg");
    thumbBar.appendChild(newImage);
    function displayThumb(clickedImage) {
        var imageThumb = clickedImage.target.getAttribute('src');
        displayedImage.setAttribute('src', imageThumb);
    };
    newImage.addEventListener('click', displayThumb);
}

While the following does not work:

for (i = 1; i <= 5; i++) {
    var newImage = document.createElement('img');
    newImage.setAttribute('src', "images\/pic" + i + ".jpg");
    thumbBar.appendChild(newImage);
    function displayThumb(clickedImage) {
        var imageThumb = newImage.getAttribute('src');
        displayedImage.setAttribute('src', imageThumb);
    };
    newImage.addEventListener('click', displayThumb);
}

In other words, what is the event object in the displayThumb function doing in the first example that allows it to be functioning on the entire loop, whereas in the second example the loop is completed, and the click events only function on the last thumbnail.


(Mittineague) #19

I know how you feel, I’m not new to JavaScript and I don’t understand it well enough to easily explain it, but it has to do with “closures” and “lexical scope”

I think the “Creating closures in loops: A common mistake” section near the end of this page does the best job at explaining that I’ve seen.


(Chris Mills) #20

Hi there @impresently!

This is a tricky aspect of JavaScript, and I don’t blame you for having trouble with it. Let’s try to clear things up.

When a function is run as a direct result of an event firing, due to an event listener being added to the object on which the event fires, the function implicitly has an event object available inside its lexical scope (i.e. inside the function, but not outside it). The reason we write e, or event, or in this case clickedImage as a parameter is to give a definition to this event object — i.e. a name we can refer to it as.

So in this case, each time the loop is run, the displayThumb() function is defined, and we add an event listener to the newImage created on each round of the loop that makes it so that displayThumb() is run when the image in question is clicked on.

By the way, it would arguably be more efficient and better practice to define the function only once, outside the loop, rather than each time it runs (see our version at https://github.com/mdn/learning-area/blob/master/javascript/building-blocks/gallery/main.js), but yours still works OK.

So the event object passed into the function each time an image is clicked (clickedImage in your case) varies depending on which image was clicked. It contains information relevant to the image that was clicked.

Its target attribute contains a reference to the image itself — “the target of the event”. This is why when you do clickedImage.target, you can directly access the properties available on image. We can therefore use getAttribute('src') to access the contents of the image’s src attribute (the path to the image), etc.

Now, we also need to talk about the order in which things are run around loops. When you run a loop and it has a function inside it, the different loop iterations all run first, then the functions inside the loop iterations will run afterwards.

This is why the second code example DOESN’T work properly. By the time the loop iterations have all run, newImage's src value is equal to the path to the last image. The event listeners are all set up correctly, but the function will always set the main image displayed to that last image.

If you instead get the src from e.target.getAttribute(), the value will be relative to the image clicked on, not the value of newImage after the loops have all run, which is always equal to the last image.

Does that help? Happy to discuss some more if you are still not clear.


#21

Hi there, I have a question regarding my version:

var displayedImage = document.querySelector('.displayed-img');
var thumbBar = document.querySelector('.thumb-bar');

btn = document.querySelector('button');
var overlay = document.querySelector('.overlay');

for(var i = 1; i < 6; i++){
  var newImage = document.createElement('img');
  newImage.setAttribute('src', "images/pic" + i + ".jpg");
  thumbBar.appendChild(newImage);
};

thumbBar.addEventListener('click', function(e){
	var imgSrc = e.target.getAttribute('src');
	displayedImage.setAttribute('src', imgSrc);
});

So I added the eventListener to “thumbBar”, rather than to its child “newImage” as in finished example, and in this way avoided mentioning it in the loop altogether. It works as intended and I spotted no errors, but I would like to know if this version is consistent with the standards and can be used alongside with yours. Thanks! :slight_smile:


(Chris Mills) #22

Hey there!

Yes, this works well, due to event bubbling. I think I didn’t implement like that originally because I wanted to keep things more obvious, but I don’t think it actually makes a lot of difference.

Nice work.


(Askaroff5131) #23

Hello everyone,

I feel so bad about it but I gave up on this assignment and looked at the example code.

The reason I gave up is because I got so confused that I could not continue any more yet I knew I was going in the right direction. I knew that I had to and how to create the loop and use the even object but I just got stuck on one part.

I am going to explain my confusion with the “looping through the images and event handler” part only as I have not yet done the darkening effect part.

  1. I found the “Steps to complete” the most confusing. For example, in the first section of the “Adding an onclick handler to each thumbnail image”, in this part - “This can be done by running the getAttribute() function on the <img> in each case …” - I could not understand why the function would run on the <img> which is as I understand a different element in the html located in the full-img div when the newImage is the one we need with a location in the thumb-bar div. My question is why is the <img> mentioned here at all when all this section wants us to do is create the following code where <img> or full-img div is not even mentioned; newImage.onclick = function(e) {

var imgSrc = e.target.getAttribute(‘src’);

Please correct me if I am wrong about this.

  1. This part - “Run a function, passing it the returned src value as a parameter. You can call this function whatever you like.” - I would have probably figured out somehow if I had been a bit more patient and not gotten stuck with the section above.
  2. The same with the third section.

Since I was doing this all on my on my Iphone, I had to store all the images on an external server online which gave me links to them. So I created an array with the links and accessed it with the loop thus assigning them as attributes to newImage. However, it does not seem to work. Can you please explain why;

Here’s my code;

for ( i = 0; i = 4; i++ ) {

var newImage = document.createElement(‘img’);

var images = [“https://s33.postimg.cc/71rt7bggv/pic1.jpg”, “https://s33.postimg.cc/4kg202bzz/pic2.jpg”,

https://s33.postimg.cc/tqh06w5kf/pic3.jpg”,

https://s33.postimg.cc/xa2xwpnpr/pic4.jpg”,

https://s33.postimg.cc/5mq8ima8v/pic5.jpg”];

newImage.setAttribute(‘src’, images[i]);

thumbBar.appendChild(newImage);

newImage.onclick = function(e) {

var imgSrc = e.target.getAttribute(‘src’);

displayImage(imgSrc);

}

}

function displayImage(imgSrc) {

displayedImage.setAttribute(‘src’, imgSrc);

Another question that still boggles me is that it seems to be okay to call a function and define it later. Not only that but also call a function inside a scope closed by braces which is the loop and define it later outside of that scope. To my knowledge, this contradicts some of the lessons on JS here. I know Chris sent a link for an explanation earlier but since the content there is a lil advanced, I could not understand it. I would appreciate a simple explanation.

By the way, could anyone let me know how long it took them to do this whole assignment? A few hours or days? I was so impatient that I looked at the example after having re-read the steps to complete 3 or 4 times and still remaining confused about it. Is it best to just leave it for a while and get back to it with a fresh mind?

Special thanks to Chrismills and everyone trying to help others who have a hard time grasping the assignment.

Thank you!!!