West Midlands | ITP-Sept-2025 | Ali Naru | Sprint 3 | Slideshow #884
Conversation
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above. |
MohammedNaru
changed the title
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above. |
MohammedNaru
added
Needs Review
There was a problem hiding this comment.
You may want to double-check the opening and closing tags, especially around the button-bar
There was a problem hiding this comment.
Also, recheck your main HTML opening tag
There was a problem hiding this comment.
Great work, but you might want to recheck how the auto-mode buttons get enabled and disabled during different actions.
eliza-an
left a comment
eliza-an
left a comment
There was a problem hiding this comment.
There is a lot very advanced javascipt code in here. It's all written really well, but I want to mention 2 things:
- It's important that your code is readable- not just simple. Would your peers be able to easily review your code? or would they have to look things up?
- It is also important that you yourself are able to understand every section of code that you've written, The features here are super advanced. Would you yourself be able to find a bug easily if it were to come up? If so excellent work!! If not- it might be good to rewrite this in a simpler way so bugs are easier to spot for you!
| @@ -1,8 +1,103 @@ | |||
| const images = [ | |||
| "./assets/cute-cat-a.png", | |||
There was a problem hiding this comment.
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!
| }); | ||
|
|
||
| function actButton(id) { | ||
| switch (id) { |
There was a problem hiding this comment.
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!
| } | ||
|
|
||
| function backward() { | ||
| index = index === 0 ? images.length - 1 : index - 1; |
There was a problem hiding this comment.
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!
eliza-an
added
Reviewed
3 participants
Slideshow Pull Reqest
Self checklist
Changelist
Changed Js, Html and Css files to meet task requirements.
Questions
No Questions for reviewer.