Skip to content
This repository has been archived by the owner on Oct 26, 2020. It is now read-only.

Week6 #1001

Open
wants to merge 25 commits into
base: scotland-class4
Choose a base branch
from
Open

Week6 #1001

wants to merge 25 commits into from

Conversation

alastair87
Copy link

Your Details

Your Name:
Your City:
Your Slack Name:

Homework Details

Module:
Week:

@alastair87 alastair87 self-assigned this Jul 1, 2020
clearInterval(interval);
}
}, 1000);
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. Note that at the moment there is a bug caused by the fact that there is nothing stopping the alarm timer being being set more than once. Is there a way you could fix it, perhaps by storing a variable which reflects whether or not the timer is active somewhere?

counter = 0;
}
},
5000,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case I don't think you need a counter variable since you are always doing the same thing every 5 seconds, i.e. randomly picking from the array of quotes.

//auto play checkbox
let autoPlay = document.querySelector("#checkbox");
autoPlay.addEventListener("CheckboxStateChange", autoPlayOn);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"CheckboxStateChange" is a specific syntax that only works in Mozilla Firefox. You can make it work in all current browsers by using "change" instead.



let images = document.querySelectorAll("img");
images.forEach((image) => image.classList.add("content-title"));


// Task 3
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could do task 3 by using querySelectorAll and an array with the birth and death dates.

img.src = images[counter % images.length].src;
counter++;
console.log(counter % images.length);
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't ideal if your slideForward() function relies on continuing to increase the value of counter like this, since eventually this would cause an exception if the number got to be so large that it exceeded the allowed size of an integer.

You could avoid this problem by doing something like this, similar to how you've dealt with it in slideBack(), except you need to do it before accessing the img.src property:

if (counter === images.length - 1) {
    counter = 0;
} else {
    counter++;
}

If you use if-else blocks like these to maintain counter then you also don't need to do counter % images.length as this value will be the same as counter.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants