-
-
Notifications
You must be signed in to change notification settings - Fork 155
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
base: main
Are you sure you want to change the base?
Changes from all commits
b06cb19
c9fbbff
95f7fd3
f02e624
c87010e
0b3bd51
5c03c87
89b9b6a
972b726
21f47be
2020a5f
1f6978e
96c0b2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,4 +12,4 @@ const recipe = { | |
|
||
console.log(`${recipe.title} serves ${recipe.serves} | ||
ingredients: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You haven't solved the problem here. |
||
${recipe}`); | ||
${recipe}`); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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", () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. solid implementation here. well done! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you! I appreciate the feedback. |
||
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(); | ||
}); |
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; | ||
} |
There was a problem hiding this comment.
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.