Skip to content

London | 25-ITP-May | Houssam Lahlah | Sprint 3 | Slide show #751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Sprint-2/debug/address.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ const address = {
postcode: "XYZ 123",
};

console.log(`My house number is ${address[0]}`);
console.log(`My house number is ${address[0]}`);

Choose a reason for hiding this comment

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

did you check your console to see the result of this console.log? You appear to be using a property reserved for arrays on an object. My log shows me "My house number is undefined" which is not the expected behaviour.

Choose a reason for hiding this comment

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

lastly, you should keep your PRs separate. this one is about the slide show project, not debug. therefore you would need to open different PRs for different projects.

2 changes: 1 addition & 1 deletion Sprint-2/debug/recipe.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ const recipe = {

console.log(`${recipe.title} serves ${recipe.serves}
ingredients:

Choose a reason for hiding this comment

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

You haven't solved the problem here.

${recipe}`);
${recipe}`);
15 changes: 13 additions & 2 deletions Sprint-3/slideshow/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,23 @@
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Title here</title>
<script defer src="slideshow.js"></script>
<title>Image carousel</title>
<link rel="stylesheet" href="style.css" />
</head>
<body>
Copy link

Choose a reason for hiding this comment

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

I ran your demo and I can see the buttons move up and down the page when the image changes to the next slide. if you had to improve the UX of this app, what would you change?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the feedback! I would improve the UX by keeping the navigation buttons fixed in a consistent position, so they don’t shift when the image changes. I’d also add smoother transition effects between slides and ensure the carousel is responsive for different screen sizes.

Choose a reason for hiding this comment

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

this is an interesting approach but I am sure there is a better way to do this. Have you tried separating the image and control buttons on the page. Perhaps having different containers for them?

<img id="carousel-img" src="./assets/cute-cat-a.png" alt="cat-pic" />

<button type="button" id="backward-btn">Backwards</button>
<button type="button" id="forward-btn">Forward</button>

<button id="auto-forward">Auto Forward</button>
<button id="auto-backward">Auto Back</button>
<button id="stop">Stop</button>

<label for="delay-input">Delay between images (seconds): </label>
<input type="number" id="delay-input" min="1" max="10" value="2" />

<script src="slideshow.js"></script>
<script src="slideshow-extra.js"></script>
</body>
</html>
60 changes: 59 additions & 1 deletion Sprint-3/slideshow/slideshow.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,63 @@ const images = [
"./assets/cute-cat-c.jpg",
];

let currentIndex = 0;
const imgElement = document.getElementById("carousel-img");
const forwardBtn = document.getElementById("forward-btn");
const backwardBtn = document.getElementById("backward-btn");

// Write your code here
const autoForwardBtn = document.getElementById("auto-forward");
const autoBackBtn = document.getElementById("auto-backward");
const stopBtn = document.getElementById("stop");


let intervalId = null; // <-- declare intervalId

function updateImage() {
imgElement.src = images[currentIndex];
}

forwardBtn.addEventListener("click", () => {
Copy link

Choose a reason for hiding this comment

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

solid implementation here. well done!

Copy link
Author

Choose a reason for hiding this comment

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

Thank you! I appreciate the feedback.
I’ll keep refining the UX to make the carousel smoother and more consistent.

currentIndex = (currentIndex + 1) % images.length;
updateImage();
});

backwardBtn.addEventListener("click", () => {
currentIndex = (currentIndex - 1 + images.length) % images.length;
updateImage();
});

function disableAutoButtons() {
autoForwardBtn.disabled = true;
autoBackBtn.disabled = true;
stopBtn.disabled = false;
}

function enableAutoButtons() {
autoForwardBtn.disabled = false;
autoBackBtn.disabled = false;
stopBtn.disabled = true;
}

autoForwardBtn.addEventListener("click", () => {
clearInterval(intervalId);
intervalId = setInterval(() => {
currentIndex = (currentIndex + 1) % images.length;
updateImage();
}, 2000);
disableAutoButtons();
});

autoBackBtn.addEventListener("click", () => {
clearInterval(intervalId);
intervalId = setInterval(() => {
currentIndex = (currentIndex - 1 + images.length) % images.length;
updateImage();
}, 2000);
disableAutoButtons();
});

stopBtn.addEventListener("click", () => {
clearInterval(intervalId);
enableAutoButtons();
});
43 changes: 43 additions & 0 deletions Sprint-3/slideshow/style.css
Original file line number Diff line number Diff line change
@@ -1 +1,44 @@
/** Write your CSS in here **/

body {
font-family: Arial, sans-serif;
text-align: center;
background-color: #f8f9fa;
margin: 2em;
}

img#carousel-img {
max-width: 400px;
height: auto;
border-radius: 10px;
box-shadow: 0 0 10px rgba(0, 0, 0, 0.2);
margin-bottom: 1em;
}

button,
input[type="number"] {
margin: 0.3em;
padding: 0.6em 1.2em;
font-size: 1em;
border-radius: 6px;
border: 1px solid #ccc;
cursor: pointer;
transition: background-color 0.3s;
}

button:disabled,
input:disabled {
cursor: not-allowed;
background-color: #eee;
color: #999;
}

button:hover:not(:disabled) {
background-color: #007bff;
color: white;
}

label {
font-weight: bold;
margin-right: 0.5em;
}