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

Pine-Roslyn M. #74

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Pine-Roslyn M. #74

wants to merge 3 commits into from

Conversation

roslynm
Copy link

@roslynm roslynm commented Dec 11, 2021

No description provided.

Copy link

@jbieniosek jbieniosek left a comment

Choose a reason for hiding this comment

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

Great work on this project! This project is green.

<section class="main-container">
<section class="controls">
<div class="subcontrols">Temperature
<div><span id="down">🧊</span><span class="tempcol" id="temp"> 70 </span><span id="up">🔥</span></div></div>

Choose a reason for hiding this comment

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

To add spacing between the span tags, look at padding-left and padding-right.

const skyscene = document.getElementById('gardenactivity')
const garden = document.getElementsByClassName('garden')

const changeTempColor = () => {

Choose a reason for hiding this comment

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

Great helper function!

Comment on lines +63 to +73
sky.addEventListener('change', changeSky)

form.addEventListener('input', changeCity)

up.addEventListener('click', ()=>{
state.tempCount += 1;
temp.textContent = `${state.tempCount}`;
changeTempColor();
})

down.addEventListener('click', () => {

Choose a reason for hiding this comment

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

I recommend wrapping all of the addEventListener calls in a registerEventHandler type function, and then set that up to run when "DOMContentLoaded" event triggers.

@@ -0,0 +1,79 @@
const up = document.getElementById("up")

Choose a reason for hiding this comment

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

This setup works, but if the script tag was moved from the bottom of the web page to the top, they would stop working, because they would attempt to run before the html loads. I would recommend grabbing these elements in the functions that need them, rather than doing it all at the same time here.

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