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

News Site #23

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

Conversation

chrisphillers
Copy link

Styling needs finishing off & infographic function unworking/unfinished.

<header class="heading">
<h1>THE END IS NIGH</h1>
<form class='form__search'>
<input class="searchClass" type="search" results=5 placeholder="Search me">
Copy link
Contributor

Choose a reason for hiding this comment

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

Clean indentation please

topNews.articles.forEach(content => {
const spanNode = document.createElement("li"); // make fresh <li>
spanNode.innerHTML = // news contents
`<a href="${content.url}">
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation here would make the HTML easier to read

const searchFunc = document.querySelector('.form__search');
searchFunc.addEventListener("submit", function(event){ //COMBINE SEARCH AND INPUT??
event.preventDefault();
console.log(event.target['0'].value); // get the first item in teh form element.
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better to get the input using a selector rather than by index. That way if HTML gets moved around it will still work.



//button function
function fetchingButtons(url, selector) { //button fetch addresses
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this function can be removed as it does not get called

})
.then(function(body){
parentNode.innerHTML = "";
displayDataOnPage(body);
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

newsSource.articles.forEach(content => {
const spanNode3 = document.createElement("li");
spanNode3.attribute

Copy link
Contributor

Choose a reason for hiding this comment

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

There are quite a few empty lines around. It would be better to remove them and keep empty lines to just one. It will make code easier to read.

@dmitrigrabov
Copy link
Contributor

Good work. Indentation and code layout needs more attention. Keeping it nicely laid out will make it easier to read. Also, I would have liked to see a README which explains what the project does, highlights any own features added etc.

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.

None yet

2 participants