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

Clayton wilkerson #1000

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

Conversation

kallen07
Copy link

Kalina is creating this pull request because Clayton's PR accidentally compared his master branch rather than claayton-wilkerson branch

@@ -0,0 +1,15 @@
{
Copy link
Author

Choose a reason for hiding this comment

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

You can add this to your .gitignore file in the future to prevent it from being pushed to github
https://www.pluralsight.com/guides/how-to-use-gitignore-file

Not at all mandatory, but nice to know

@@ -30,13 +30,22 @@ Edit this document to include your answers after each question. Make sure to lea

1. What is the DOM?

Document object module: the representation of the content from html, css, and js.
Copy link
Author

Choose a reason for hiding this comment

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

the object representation*

3. What is an event listener?

A way to track events and controll what happens after an event is triggered
Copy link
Author

Choose a reason for hiding this comment

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

event listeners are functions that execute when a relevant event fires

4. Why would we convert a NodeList into an Array?

Nodelist do not have the same methods as an array, sometimes you might want to apply an array method to a nodelist.
Copy link
Author

Choose a reason for hiding this comment

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

yes, such as .map()

.then((res) => {
console.log ("Res: ", res)
articlesArray = res.data.articles;
let javascriptArt = articlesArray.javascript;
Copy link
Author

Choose a reason for hiding this comment

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

You can loop through each of the topics (javascript, bootstrap, etc) using Object.entries

Here's a link explaining more. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/entries

This isn't something you'd be expected to know but it's a good coding style adjustment.

You can also read about how to use a for loop to accomplish the same thing https://stackoverflow.com/questions/684672/how-do-i-loop-through-or-enumerate-a-javascript-object#:~:text=You%20can%20use%20the%20for,t%20come%20from%20the%20prototype.&text=Using%20the%20new%20Object.




function Cards(article) {
Copy link
Author

Choose a reason for hiding this comment

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

Since this function is being defined in the global scope, it should be aligned with the axios.get( request and variable definitions above. Otherwise it looks, at a glance, like this function is being defined inside the .catch() or .then() functions (which we wouldn't want).

Summary: this shouldn't be indented.

@@ -11,4 +11,27 @@
// Use your function to create a header
// and append it to the DOM inside the div.header-container

function Header() {}
function Header(date,title,temp) {
let header0 = document.createElement('div');
Copy link
Author

Choose a reason for hiding this comment

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

I would recommend using const here because you aren't re-assigning any of these objects, you are only changing their attribute values.

Not something you are expected to know but it's good coding practice!

Choose a reason for hiding this comment

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

I've developed a bad habit of just using let. I'll practice being cognizant of when to use each.

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