To be reviewed#16
Conversation
Level 100 done
100 complete
lvl-100 complete
350 completed
responsive design
responsive design
responsive design
responsive tab screen
responsive tab screen
responsive tab screen
search show and select show added
search show and select show designs
CSS designs for the showlists
search functionality extended
completed level 500
✅ Deploy Preview for cyf-janefrancess-tv ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
RobCso
left a comment
RobCso
left a comment
There was a problem hiding this comment.
Great job, especially on the visual side, you have a few bugs on your website but they can be fixed easuly
| season = season < 10 ? "0" + season : season; | ||
| number = number < 10 ? "0" + number : number; | ||
| return `S${season}E${number}`; | ||
| } |
There was a problem hiding this comment.
Very good helper function.
| align-items: center; | ||
| gap: 1rem; | ||
| margin: auto; | ||
| } |
There was a problem hiding this comment.
nice to see media queries to make the site responsive. there is slight issue with break points as the site doesn't break immediately after hitting the element but goes on to cover it for a bit before it breaks, but its a simple fix
| return response.json(); | ||
| } else { | ||
| throw new Error( | ||
| `Encountered something unexpected: ${response.status} ${response.statusText}` |
There was a problem hiding this comment.
Good to see your fetch api filters the network response and throws an error when something is wrong
| const showRuntime = document.createElement("p"); | ||
| showRuntime.innerHTML = `<strong style = "color: green">Runtime: </strong>${show.runtime}`; | ||
| comboEl.appendChild(showRuntime); | ||
|
|
There was a problem hiding this comment.
really like how you indented your function and commented on each block, this makes your code easily readable
| homeEl.style.display = "block"; | ||
| document.querySelector(".search-icon").style.display = "block"; | ||
| displayCountEl.innerHTML = `Displaying ${selectedShow.length}/${allShows.length} show(s)`; | ||
| }); |
There was a problem hiding this comment.
good use of the filter method with the eventlistener
| episodeSummary.className = "episode-summary"; | ||
| episodeContainer.appendChild(episodeSummary); | ||
| // console.log(allShows); | ||
| }); |
There was a problem hiding this comment.
good use of the forEach method and like the readability
Know-Thyself
left a comment
Know-Thyself
left a comment
There was a problem hiding this comment.
Your TV app is fully functional and looks great as well.
I'm impressed by your attention to details!
Great work!
|
|
||
| // all shows | ||
|
|
||
| const loadShowList = (showID) => { |
There was a problem hiding this comment.
Well written function to fetch all episodes of a show! However, you may want to rename the function loadShowList to match what it does, (e.g. loadEpisodes).
| }); | ||
| }; | ||
|
|
||
| function displayAllShows(showList) { |
There was a problem hiding this comment.
Great function to populate your home page! It's well commented and clear to follow what happens at each step. To make it even better, think of refactoring it by moving some codes to helper functions so that it doesn't have to deal with everything.
| "https://img.freepik.com/premium-vector/error-404-landing-page-with-file-flat-design_249405-162.jpg?w=2000"; | ||
| showContainer.appendChild(showImage); | ||
| // show ratings combo | ||
| const comboEl = document.createElement("section"); |
There was a problem hiding this comment.
Creating this comboEl container shows that you think of your web page's layout in advance.
Well done!
| showSummary.className = "show-summary"; | ||
| showSummary.innerHTML = `${show.summary}`; | ||
| expandMoreContent.appendChild(showSummary); | ||
| // const expandMoreHolder = document.createElement("section"); |
There was a problem hiding this comment.
Nice try here on the commented out code to create read more and read less functionality! Although, you still can make it work using JavaScript, there's also a purely CSS and a lot easier solution to it.
Check this YouTube video out: https://www.youtube.com/watch?v=OhCzEjXvY9A&list=LL&index=4
Or if you would like to do it with JavaScript, think of updating only the innerHTML to truncated or full summary. You can also switch the innerHTML of the same button between read more and read less following some simple logic.
| }); | ||
| // console.log(searchedShow); | ||
| containerEl.innerHTML = ""; | ||
| selectShowEl.innerHTML = ""; |
There was a problem hiding this comment.
Great work on updating select options based on the search filter.
|
|
||
| // using the API fetch | ||
|
|
||
| const setup = async () => { |
There was a problem hiding this comment.
Well written asynchronous setup function!
Moving it up to somewhere like line 7 would be a good idea as it is the entry point to your application.
| }; | ||
|
|
||
| // displaying the data contents | ||
| function makePageForEpisodes(episodeList) { |
There was a problem hiding this comment.
Like the shows side, good job on episodes main function as well!
3 participants
No description provided.