Skip to content

Soph weather app complete#11

Open
so-chow wants to merge 1 commit intowdi-atx-12:masterfrom
so-chow:weatherapp_hw
Open

Soph weather app complete#11
so-chow wants to merge 1 commit intowdi-atx-12:masterfrom
so-chow:weatherapp_hw

Conversation

@so-chow
Copy link

@so-chow so-chow commented Sep 27, 2017

No description provided.

Copy link

@BritneyJo BritneyJo left a comment

Choose a reason for hiding this comment

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

Next time you can google for a CDN for the flag library instead of downloading and using node to install. That will eliminate having 500+ files you have to add to the repo: https://cdnjs.com/libraries/flag-icon-css

@@ -4,48 +4,57 @@
<meta charset="utf-8">
<title>Mini Weather App</title>
<link rel="stylesheet" href="css/style.css">

Choose a reason for hiding this comment

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

Make sure your custom CSS file is loaded after any libraries you may be using (i.e. boostratp, googlefonts, flags)

<div class="row form-row">
<div class="col-md-4 col-md-offset-4">
<div class="header">
<div class="weat">

Choose a reason for hiding this comment

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

Confusing class name... While it definitely works, it's not super clear to another dev.

$(function(){
console.log("Listo!");
requestWeatherData();
$("form").submit(function(event){

Choose a reason for hiding this comment

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

watch your indents!

})
}

function onSuccess(data){

Choose a reason for hiding this comment

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

You'll want to make sure to clear out the text input after a successful data display too!

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