-
Notifications
You must be signed in to change notification settings - Fork 34
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
My news reader #20
base: master
Are you sure you want to change the base?
My news reader #20
Conversation
…d article template
…eds some serious polish still
…that brings in. Also decided it was about time to put a little formatting into the date. And also using the 100 results to populate a sorted list of top ten publishers for the query
} | ||
|
||
// api request to news api. Returns json and calls createArticles function | ||
const getArticles = (page=1, excludedDomainsStr='', filterDomains="") => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
twentyArticles = hundredArticles.slice(startArticle,startArticle+20); | ||
itemId = 0; | ||
twentyArticles.forEach(article => { | ||
itemId ++ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can get the itemId using forEach index for example twentyArticles.forEach((article, index)){
} | ||
|
||
// add a listener to article creating 'click' event that takes user to URL | ||
const clickArticle = (articleNode, url) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it not be easier to wrap the html with an <a>
tag?
|
||
|
||
// adds listeners that change the colour of the article the mouse is over - move to css | ||
const mouseOverArticle = (articleNode) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be easier to achieve using CSS :hover
It would be great to see an updated README so other people looking at the repo in future can see what you have done, how it works, what technologies it uses |
BTW, if you don't need old.js, new.js and functionNames.js, you can delete them, commit and push |
Please ignore old.js, new.js and functionNames.js .
Thanks!
Phil