accidentally closed the previous pull request #29
accidentally closed the previous pull request #29KateMhq wants to merge 19 commits intoconstructorlabs:masterfrom
Conversation
| </head> | ||
| <body> | ||
| <h1>My Favorites</h1> | ||
| <!-- <form class="search-area"> |
There was a problem hiding this comment.
Commented out code can be removed
| //this is to insert search results using dom | ||
| function searchResult(body) { | ||
| body.Search.forEach(movie => { | ||
| let searchResultContainer = document.createElement("li"); |
There was a problem hiding this comment.
It might be easier to create elements using HTML string templates rather than using createElement for each item
|
|
||
| let favorite = document.createElement("button"); | ||
| favorite.innerHTML = "Add to favorites"; | ||
| favorite.addEventListener("click", function(event) { |
There was a problem hiding this comment.
It would be better to use event delegation. Let the events bubble up and handle them in one place rather than place place event handlers on elements individually
| }); | ||
| } | ||
|
|
||
| searchSubmit.addEventListener("click", function(event) { |
There was a problem hiding this comment.
We want to listen for a submit event on the form rather than a click on submit button
| .then(response => response.json()) | ||
| .then(body => { | ||
| if (typeof body.Search !== "undefined") { | ||
| let arr=body.Search; |
There was a problem hiding this comment.
there is a fair bit of code duplication here. We could instead use a loop here to perform same task 3 times
| } | ||
|
|
||
|
|
||
| .search-area{ |
There was a problem hiding this comment.
Would have been great to see more BEM style CSS selectors in use
|
|
||
|
|
||
| .search-area{ | ||
| display: grid; |
There was a problem hiding this comment.
Love seeing grid being used :)
2 participants
No description provided.