Mel: Responsive News Reader #22
Conversation
|
|
||
| <div class="container"> | ||
| <main> | ||
| <nav class="news--filter"> |
There was a problem hiding this comment.
Class name should be written using __ since it indicates an element rather than modifier
|
|
||
| function displayDataOnPage(newsStories) { | ||
| const newsArray = newsStories.articles; | ||
| console.log(newsStories.articles); |
There was a problem hiding this comment.
There are quite a few consol.logs left in. Once you are happy with the functionality, it's best to remove them to keep console output clean
| newsitem.source.name !== null | ||
| ? `<span class="${newsitem.source.name}"></span>` | ||
| : ""; | ||
| const url = newsitem.url !== null ? `${newsitem.url}` : "#"; |
| : ""; | ||
|
|
||
| return ` | ||
| <a href="${url}" class="news--article-link"> |
There was a problem hiding this comment.
Some indentation here would make HTML easier to read
|
|
||
| return redcardCheckbox.forEach(item => { | ||
| item.addEventListener("change", function(event) { | ||
| event.target.checked |
There was a problem hiding this comment.
This would be cleaner written as event.target.parentNode.style.backgroundColor = event.target.checked ? "red" :""
| const defaultArray = ["daily-mail", "mirror", "metro"]; | ||
|
|
||
| // create an array from object using key values | ||
| let publicationArray = Object.keys(publicationList); |
There was a problem hiding this comment.
if variables will not be re-assigned to in future they should be declared using const
| input.addEventListener("change", function(event) { | ||
| // new assigned value to match object key | ||
| // assign object value if checked is true | ||
| if (event.target.checked === true) { |
There was a problem hiding this comment.
This could be shortened to publicationList[event.target.value] = event.target.checked
|
It would be great to see a README which explains what the projects, how it works, what it does etc. This could be useful if you want to show your projects with potential employers. It could also be a n opportunity to highlight any stretch goals |
|
Also, if the javascript-workings js files are not needed any more, they can be deleted so they don't clog up source control |
2 participants
Still working on click event for checkbox added to each article to change article (parentNode) style.