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

Paper - Kate Malakhova #59

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

Paper - Kate Malakhova #59

wants to merge 6 commits into from

Conversation

kmalakhova
Copy link

No description provided.

Copy link

@CheezItMan CheezItMan 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 Kate, you hit all the learning goals here. I made a couple of minor suggestions, but otherwise this is great work. The site looks very well done. I like the use of CSS Grid!

<p><button id="increase-temp">⬆</button></p>
<p><button id="decrease-temp">⬇</button></p>
</div>
<p>&nbsp;</p><span id="currentTemp">65</span>

Choose a reason for hiding this comment

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

I would start off the span here with the default class value for 65 degrees.

</header>
<body>
<section class="temperature-section">
<h2>Temperature (F)</h2>

Choose a reason for hiding this comment

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

Fahrenheit? Sigh, no one here uses rational measurements. (not serious)

@@ -0,0 +1,103 @@
let currentTemp = 65;

const updateTempColor = currentTemp => {

Choose a reason for hiding this comment

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

Good helper function

Comment on lines +27 to +35
const increaseTemp = () => {
currentTemp += 1;
updateTemp(currentTemp);
};

const decreaseTemp = () => {
currentTemp -= 1;
updateTemp(currentTemp);
};

Choose a reason for hiding this comment

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

Just a note that you can combine these functions with:

Suggested change
const increaseTemp = () => {
currentTemp += 1;
updateTemp(currentTemp);
};
const decreaseTemp = () => {
currentTemp -= 1;
updateTemp(currentTemp);
};
const changeTemp = (increment) => {
currentTemp += increment;
updateTemp(currentTemp);
};

Then we can register the event handler with an anonymous function like this:

increaseTempButton.addEventListener("click", () => updateTemp( +1 ) );

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