Glasgow | 26-ITP-May | Niangh Ciang | Sprint 3 | Project-TV-Show Level 500 - #65
Glasgow | 26-ITP-May | Niangh Ciang | Sprint 3 | Project-TV-Show Level 500#65Niangh-Ciang wants to merge 44 commits into
Conversation
Level 100 edit
Glasgow | 26-ITP-May | Sandani Kannangara | Sprint 2 | Refactor episode rendering into reusable function
Glasgow | 26-ITP-May | Sandani Kannangara | Sprint 2 | Add live episode search
Glasgow | 26-ITP-May | Sandani Kannangara | Sprint 2 | feat: add episode dropdown selector
Glasgow | 26-ITP-May | Sandani Kannangara | Sprint 2 | Refactor episode dropdown selector
remove episodes.js and switch to TVMaze API
Make a separate function for filteredEpisodes
Glasgow | 26-ITP-May | Sandani Kannangara | sprint 3 | Add dynamic show and episode selection
Glasgow | 26-ITP-May | Sandani Kannangara | Sprint 3 | Handle episodes without images
Level 400 refactor
LonMcGregor
left a comment
There was a problem hiding this comment.
- Good use of caching
- The code is organised Ok, but I have suggestions in some comments
- The deployed page has all the necessary functionality
- You have used some semantic HTML
Throughout your app's lifecycle you manually handle the styling to hide/show things element by element. Can you think of any other ways of doing this that would reduce the amount of styling done in the js?
Good work on this. ITP is complete now, but if you want an extra challenge, feel free to respond to my comments.
| placeholder="Enter genre or keyword..." | ||
| /> | ||
| <label id="genre-count">Found 0 shows</label> | ||
| <select id="genre-show-select"></select> |
There was a problem hiding this comment.
Is this select menu properly labelled? Does the lighthouse tool have any suggestions?
|
|
||
| const elements = {}; | ||
|
|
||
| function setup() { |
There was a problem hiding this comment.
The setup function is very long. Is there a way it could be split up into more task specific functions?
|
|
||
| const img = document.createElement("img"); | ||
| img.src = image?.medium || ""; | ||
| img.alt = name; |
There was a problem hiding this comment.
Is this an appropriate use of an alt tag? See what https://axesslab.com/alt-texts/ has to say
| body.className = "episode-body"; | ||
|
|
||
| const summaryP = document.createElement("p"); | ||
| summaryP.innerHTML = summary; |
There was a problem hiding this comment.
This isn't wrong, but can you think of any potential side effects of inserting the summary from the API directly into your HTML like this?
| }); | ||
| } | ||
|
|
||
| function updateGenreShowOptions(shows) { |
There was a problem hiding this comment.
be careful of the naming choices you make. What does it mean by "genre shows" in the function name here?
|
|
||
| const term = elements.genreSearch.value.trim(); | ||
|
|
||
| if (term !== "") { |
There was a problem hiding this comment.
Remember when we were discussing about simplifying code. Is this if block the simplest it needs to be?
Self checklist
Changelist
Completed all Level 500 requirements.
I added a front‑page shows listing, implemented click‑to‑open show navigation, updated the show selection UI, added free‑text show search, ensured episode search and selector work across view switches, and cached episodes to avoid repeated fetches.
##Deployed site on Netlify
https://cyf-niangh-ciang-tv.netlify.app/
This project was completed together with my partner, SandzSoft.