Skip to content

Eval #3#215

Open
JoeCarnahan42 wants to merge 8 commits intoprojectshft:masterfrom
JoeCarnahan42:master
Open

Eval #3#215
JoeCarnahan42 wants to merge 8 commits intoprojectshft:masterfrom
JoeCarnahan42:master

Conversation

@JoeCarnahan42
Copy link

No description provided.

@@ -0,0 +1,25 @@
.app-header {

Choose a reason for hiding this comment

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

some classes here are not needed and you could have used bootstrap for them
example mt-5 (margin-top) and w-auto (fit content)

alert('Error: You must enter a city name to seach.')
} else {
getLocation(cityInput.value);
cityInput.value = '';

Choose a reason for hiding this comment

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

good practice!


// City Submission //
document.querySelector('#submit').addEventListener('click', function () {
const cityInput = document.querySelector('#city-input');

Choose a reason for hiding this comment

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

here you need to trim it to remove empty spaces

Choose a reason for hiding this comment

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

also you could have done the lower case 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