Skip to content

West Midlands | ITP-Sept-2025 | Ali Naru | Sprint 3 | Slideshow #884

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.

Sign up for GitHub

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
MohammedNaru wants to merge 2 commits into
base: main
Choose a base branch
from
Open

West Midlands | ITP-Sept-2025 | Ali Naru | Sprint 3 | Slideshow #884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
728ba81
Refactor: update slideshow structure and add button functionality
MohammedNaru Nov 24, 2025
798ad1f
Merge branch 'main' into Sprint-3-Slideshow
MohammedNaru Nov 24, 2025
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
14 changes: 6 additions & 8 deletions Sprint-3/slideshow/index.html
View file
Open in desktop
Copy link

@Pappyjay157 Pappyjay157 Nov 26, 2025

Choose a reason for hiding this comment

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

You may want to double-check the opening and closing tags, especially around the button-bar

Copy link

@Pappyjay157 Pappyjay157 Nov 26, 2025

Choose a reason for hiding this comment

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

Also, recheck your main HTML opening tag

Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
<!DOCTYPE html>
<html lang="en">
<head>
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Title here</title>
<title>Image carousel</title>
<link rel="stylesheet" href="style.css" />
<script defer src="slideshow.js"></script>
</head>
<body>
<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>
</body>
</html>
<div id="button-bar"/>
</body>
</html>
103 changes: 99 additions & 4 deletions Sprint-3/slideshow/slideshow.js
View file
Open in desktop
Copy link

@Pappyjay157 Pappyjay157 Nov 26, 2025

Choose a reason for hiding this comment

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

Great work, but you might want to recheck how the auto-mode buttons get enabled and disabled during different actions.

Original file line number Diff line number Diff line change
@@ -1,8 +1,103 @@
const images = [
"./assets/cute-cat-a.png",
Copy link

@eliza-an eliza-an Dec 22, 2025

Choose a reason for hiding this comment

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

Good work! But the instructions say that we must cycle through at least 4 images and here you have 3! Make sure to read the instructions clearly!

"./assets/cute-cat-b.jpg",
"./assets/cute-cat-c.jpg",
"./assets/cute-cat-a.png",
"./assets/cute-cat-b.jpg",
"./assets/cute-cat-c.jpg",
];

const buttons = [
{
type: "button",
id: "auto-backward",
text: "Auto Back",
},
{
type: "button",
id: "backward-btn", // updated to match test
text: "Back",
},
{
type: "button",
id: "stop",
text: "Stop",
},
{
type: "button",
id: "forward-btn", // updated to match test
text: "Forward",
},
{
type: "button",
id: "auto-forward",
text: "Auto Forward",
},
];

const buttonBar = document.getElementById("button-bar");
const imageElement = document.getElementById("carousel-img");

let index = 0;
let intervalId = null;

// Set initial image
imageElement.src = images[index];

// Create buttons and add event listeners
buttons.forEach((item) => {
const button = document.createElement("button");
button.type = item.type;
button.id = item.id;
button.textContent = item.text;
button.addEventListener("click", () => actButton(item.id));
buttonBar.appendChild(button);
});

function actButton(id) {
switch (id) {
Copy link

@eliza-an eliza-an Dec 22, 2025

Choose a reason for hiding this comment

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

this is a good use of the switch case statements, could you explain to me what this does and why you chose to use it instead of an if else? good knowledge exersise!

case "auto-backward":
clearInterval(intervalId);
disableAutoButtons();
intervalId = setInterval(backward, 2000);
break;

case "backward-btn":
backward();
break;

case "stop":
clearInterval(intervalId);
enableAutoButtons();
break;

case "forward-btn":
forward();
break;

case "auto-forward":
clearInterval(intervalId);
disableAutoButtons();
intervalId = setInterval(() => {
forward();
}, 2000);
break;
}
}

function disableAutoButtons() {
document.getElementById("auto-forward").disabled = true;
document.getElementById("auto-backward").disabled = true;
}

function enableAutoButtons() {
document.getElementById("auto-forward").disabled = false;
document.getElementById("auto-backward").disabled = false;
}

function backward() {
index = index === 0 ? images.length - 1 : index - 1;
Copy link

@eliza-an eliza-an Dec 22, 2025

Choose a reason for hiding this comment

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

This is a good section of code but it's not very readable. As a programmer, it's importatnt to write code that a reviewer can understand at first glance without thinking too hard about it.
Could you try to rewrite this in a more readable way? It would also be helpful to understand exactly what this section does. Could you reply to me in this thread and explain it to be like i am a beginner? Another good learning exercise!

imageElement.src = images[index];
}

// Write your code here
function forward() {
index = (index + 1) % images.length;
imageElement.src = images[index];
}
27 changes: 27 additions & 0 deletions Sprint-3/slideshow/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,28 @@
/** Write your CSS in here **/
body {
object-fit: contain;
display: block;
}

#carousel-img {
width: 100%;
max-width: 400px;
height: auto;
display: block;
margin: 20px auto;
border-radius: 10px;
object-fit: contain;
box-shadow: 0 4px 8px rgba(0, 0, 0, 0.2);
}

#button-bar {
display: flex;
justify-content: center;
flex-wrap: wrap;
gap: 10px;
margin-top: 20px;
}

button {
color: blue;
}