Conversation
Project tv level 100
LONDON | ITP-May-26 | Damilola Odumosu | Sprint 2 | Project TV Show
Level 300: Fetch episodes from TVMaze API
London | 26-ITP-May | Damilola Odumosu | TV Show project | Level 400
ChinweP
added
Needs Review
ChinweP
added
the
Needs Review
|
Netlify Link: https://deploy-preview-5--cyf-chinwep-tv.netlify.app/ |
LonMcGregor
added
the
Review in progress
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
- Good use of semantic HTML
- Caching is appropriate
- Functionality works well
Good work on this, I have left some comments to consider if you want a bit more practice now ITP is over.
| ); | ||
| }); | ||
|
|
||
| episodeCount.textContent = |
There was a problem hiding this comment.
Is this counting feature also implemented for shows?
| showImage.src = show.image.medium; | ||
| } | ||
|
|
||
| showImage.alt = show.name; |
There was a problem hiding this comment.
Is this an appropriate use of an alt tag? See if https://axesslab.com/alt-texts/ has any suggestions
|
|
||
| option.value = episode.id; | ||
|
|
||
| const episodeCode = |
There was a problem hiding this comment.
This episode number code is repeated, is there a useful refactoring you could do here?
|
|
||
| statusMessage.textContent = "Episodes loading..."; | ||
|
|
||
| if (episodesCache.has(show.id)) { |
There was a problem hiding this comment.
When we consider functions focusing on the right functionality, is it more appropriate to check the cache here in showEpisodes, or in the getShowEpisodes function?
LonMcGregor
added
Reviewed
This branch has not been deployed
3 participants
Learners, PR Template
Self checklist
Changelist
Completed Level 500 according to requirements.
Netlify Link: https://deploy-preview-5--cyf-chinwep-tv.netlify.app/
Partner: Damilola Odumosu https://github.com/d-odumosu