Skip to content

cedar mac#71

Open
mac-madison wants to merge 2 commits intoAda-C16:mainfrom
mac-madison:main
Open

cedar mac#71
mac-madison wants to merge 2 commits intoAda-C16:mainfrom
mac-madison:main

Conversation

@mac-madison
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@beccaelenzil beccaelenzil left a comment

Choose a reason for hiding this comment

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

Nice work giving this project your own flavor! It's a really cool dynamic page and it's clear you did lots of learning. One of the learning goals of this project was to research how to detect and respond to events for an input and select element. I encourage you to do that research even if those elements aren't on your page.

Comment thread src/index.js
Comment on lines +20 to +22
let div = document.createElement('div');
div.className = 'content';
let h3 = document.createElement('h3');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In general we should use const for elements since we will not overwrite these, but modify their attributes

Comment thread src/index.js
let section = document.createElement('section');
section.className = 'container';

if (i < 45) section.style.color = 'rgb(103, 128, 147)';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For consistency, consider sticking with applying classNames to apply styles

Comment thread src/index.js
}

/* a function to create the temps,
this is the only function syntax i could get to work?
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm not sure why this is the case

Comment thread src/index.js
document.body.style.backgroundColor = 'black';
}

/* The code above is responsible for creating the numbers/sections for numbers
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I noticed that you added your onclick behavior directly to your html rather than adding event listeners in your javascript. Is there a reason you made this choice.

Comment thread src/index.js
/* The code above is responsible for creating the numbers/sections for numbers
and changing the colors/videos from radish to mint. The code below is responsible for
the scrolling effects. In order for both sections to work I had to place them
at different places in the html file. I'm not experienced enough to know why
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I also don't have a lot of experience with vanilla javascript. Let's talk more about who would be a good resource for you!

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