Skip to content

Conversation

@scarabeo7
Copy link
Owner

Hi guys, could you please review my tv project. I have done levels 100 to 400. Working on level 500

@scarabeo7 scarabeo7 requested a review from cbaggini February 28, 2021 15:14
Copy link
Collaborator

@JustinWingChungHui JustinWingChungHui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Level 350

When your page loads, it must load the episodes (for the SAME show) from TVMaze API
Good
 
Your search and episode selector must continue to work
Good
 
Your page MUST NOT re-fetch the episodes every time the user types a character into your search field
Good
 

Level 400

Add a select input which allows you to choose which show you are interested in
Good - but when the page loads, it shows all 'Select a show' but displays episodes from '24'.  Maybe this should have 24 alread selected on load
 
When a show is selected, your app should display the episodes for that show
OK but  your code errors when there are images missing.  E.g. Batman Beyond on line 35 You also show print 'null' if the summary is null. It should show not text instead e.g. when selecting the show 'Кухня'. 
 
 
Ensure that your search and episode selector controls still work correctly when you switch shows.
OK - you should clear the search box when you switch shows, or apply the search if there is something typed in there when the user switches shows For 'Кухня', the search doesn't work when summary is null
 
This show select must list shows in alphabetical order, case-insensitive.
Good

Good use of functions
Clearly formatted code
You could probably add some more comments to help a reader to understand some of the code

@cbaggini
Copy link
Collaborator

cbaggini commented Mar 2, 2021

Not much to add to Justin's review. However, I noticed that one show (Cosmos) in the list is repeated twice; one of the show id is incorrect and can't be fetched. You may want to add some code to deal with this error (or fetch the show list from the API!)

@scarabeo7 scarabeo7 requested a review from nirmeet-baweja March 3, 2021 23:13
@scarabeo7 scarabeo7 requested a review from liewteh March 5, 2021 20:51
@liewteh
Copy link
Collaborator

liewteh commented Mar 5, 2021

Very nice.
Keep up the good work

Copy link
Collaborator

@JustinWingChungHui JustinWingChungHui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Level 100

  • All episodes must be shown

    Good

  • For each episode, AT LEAST following must be displayed:

    • the episode's name - Good
    • the season number - Good
    • the episode number - Good
    • the episode's medium-sized image - Good
    • the episode's summary text - Good
  • You should combine season number and episode number into an episode code:
    Good

  • Your page should state somewhere that the data has (originally) come from TVMaze.com, and link back to that site (or the specific episode on that site). See tvmaze.com/api#licensing.

    Good

Level 200

  • Add a "live" search input:

    • Only episodes whose summary OR name contains the search term should be displayed - Good
    • The search should be case-insensitive - Good
    • The display should update immediately after each keystroke changes the input. - Good
    • Display how many episodes match the current search - Good
    • If the search box is cleared, all episodes should be shown. - Good

Level 300

  • Add a select input which allows you to jump quickly to an episode:

    • The select input should list all episodes in the format: "S01E01 - Winter is Coming" - Good
    • When the user makes a selection, they should be taken directly to that episode in the list - Good
    • Bonus: if you prefer, when the select is used, ONLY show the selected episode. If you do this, be sure to provide a way for the user to see all episodes again. - Good

Level 350

  • When your page loads, it must load the episodes (for the SAME show) from TVMaze API, using fetch, NOT from the provided getAllEpisodes function. (See below for the API "endpoint" (URL) to fetch.) - Good
  • Your search and episode selector must continue to work as specified in level 300. --good
  • Your page MUST NOT re-fetch the episodes every time the user types a character into your search field! - Good

Level 400

  • Add a select input which allows you to choose which show you are interested in

    • When a show is selected, your app should display the episodes for that show as per the earlier levels of this challenge, except that it should first fetch the episode list from the API - Good

    • Ensure that your search and episode selector controls still work correctly when you switch shows. -Good but the search stops working when there is no summary egg. Кухня (line 164)

    • This show select must list shows in alphabetical order, case-insensitive. - Good

Level 500

  • When your app starts, present a listing of all shows ("shows listing") - Good

    • For each show, you must display at least name, image, summary, genres, status, rating, and runtime. - Good
  • When a show name is clicked, your app should:

    • fetch and present episodes from that show (enabling episode search and selection as before) - Good
    • hide the "shows listing" view. -Good
  • Add a navigation link to enable the user to return to the "shows listing"

    • When this is clicked, the episodes listing should be hidden
      - The show search box still contains the previous search, but the shows are not filtered. So either clear the search box or filter the shows
  • Provide a free-text show search through show names, genres, and summary texts. - Good

  • Ensure that your episode search and episode selector controls still work correctly when you switch from shows listing to episodes listing and back. - Good

Level 999

  • Responsive design - Good, but the top overlay covers over the title on certain screen sizes on the episodes screen

Overall comments

  • Works nicely

  • Code is reasonably well commented

  • Good use of functions

  • Nice styling

  • Well named functions and variables

  • You have a setup function, but have put setup code around other parts of the code which can be confusing

Completeness 8/10
Code Style 4/5
Clarity 4/5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants