London | 26-ITP-May | Gideon Defar | Sprint 3 | Project-TV-Show Level-500 - #95
London | 26-ITP-May | Gideon Defar | Sprint 3 | Project-TV-Show Level-500#95gideondefar wants to merge 19 commits into
Conversation
London | ITP-May-26 | Gideon Defar | Sprint 1 | Project TV Show- feature/level-100
London | 26-ITP-May | Gideon Defar | Sprint 3 | Project-TV-Show Level-300
London | 26-ITP-May | Gideon Defar | Sprint 3 | Project-TV-Show Level-300-API
London | ITP-May-26 | Vito Moratti | Sprint 3 | Project TV Show/level-400
London | 26-ITP-May | Gideon Defar | Sprint 3 | Project-TV-Show Level-500
gideondefar
added
the
Needs Review
LonMcGregor
added
the
Review in progress
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Good work on this. The functionality is all implemented, and appropriate caching is used.
I did notice that your decision to show an item, rather than navigate to it's items directly, when it is selected from a dropdown, causes some usability issues. e.g. search for a term, then pick a show that doesn't have that term in it.
Your code is well written, though I feel your formatting is maybe cutting the lines off a little early.
I've spotted a few things, so if you want an extra challenge now ITP is over, see if you can address them.
| <body> | ||
| <div id="root"> | ||
| </div> | ||
| <section id="shows-view"> |
There was a problem hiding this comment.
Good work trying to use semantic HTML,but there's a few issues in how you are using it. Have a look at https://validator.w3.org/nu/?doc=https%3A%2F%2Fgideondefar.github.io%2FProject-TV-Show%2F to get some pointers
| function getImage( | ||
| image, | ||
| fallback = | ||
| "https://via.placeholder.com/300x450?text=No+Image" |
There was a problem hiding this comment.
When I try to check this site, it says it doesn't load. What is this site meant to do here?
| `; | ||
|
|
||
| const sortedShows = [...shows].sort((a, b) => | ||
| a.name.localeCompare(b.name, undefined, { |
There was a problem hiding this comment.
Can you explain how you use these optional arguments for the localCompare method?
| article.innerHTML = ` | ||
| <img | ||
| src="${image}" | ||
| alt="${escapeHtml(show.name)} poster" |
There was a problem hiding this comment.
Is this the best way to use this alt tag? See what https://axesslab.com/alt-texts/ has to say
LonMcGregor
added
Reviewed
3 participants
Self checklist
Changelist
Implemented the Level 500 shows listing and accessibility improvements.
Added a shows listing view using a responsive 2-column grid layout.
landmark.Added dual-mode search for shows and episodes.
Added back navigation between the shows listing and episode view.
Implemented in-memory API response caching to avoid unnecessary repeated fetches.
Improved accessibility by adding appropriate ARIA labels and a semantic
Improved colour contrast to address Lighthouse accessibility issues.
Tested the application locally and verified the deployed version.
Link to my live website: https://tv-show-pro.netlify.app/
Link to my partner's live website: https://vmoratti.github.io/Project-TV-Show/