Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Luke Sikuade Project Cinema #24

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ljsikuade
Copy link

No description provided.

@@ -1,48 +1,20 @@
# Project Cinema
## Cinema App
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice

});

const refineUrl = search => {
let pageNum = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't pageNum be reset to 1 every time this function is called this making pageNum++ redundant


const header = document.querySelector("header");
header.appendChild(next);
next.addEventListener("click", function(event) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would better to implement the click handler using delegation to avoid having to reset it each time function is called

const review = await getReview(movieID);
//Currently not operating as intended.
let mobileClose = "";
screen.width < 700 ? (mobileClose = "Close") : (mobileClose = "x");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could better written as const mobileClose = screen.width < 700 ? "Close" : "x". A ternary is an expression and returns a value which we can assign to output variable

const movieData = await response.json();

const movie = movieData.results.find(item => {
if (item.title === filmObject.Title) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since find callback expects a boolean, this could be simplified to return item.title === filmObject.Title

}
}

// function youMayAlsoLike(genre, actors, director, writer){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Commented out code can removed to avoid clutter

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.

2 participants