Skip to content

Integrations #428

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

Merged
merged 6 commits into from
Jul 6, 2017
Merged

Integrations #428

merged 6 commits into from
Jul 6, 2017

Conversation

jwass91
Copy link
Contributor

@jwass91 jwass91 commented Jun 28, 2017

Added the simple getData() function to Woof and then gave detailed documentation for use of several APIs.

Added documentation for the following:

  • Firebase
  • Local Storage
  • General Get Requests
  • Currency Conversion
  • Sunrise/Sunset Data
  • Scientific Calculator
  • Trivia
  • Weather
  • Financial Data (currently having CORS issues)
  • Quotes (currently having CORS issues)
  • Jokes (currently having CORS issues)

@stevekrouse
Copy link
Owner

[This is an automated integration to preview this pull request's changes to the website.]

https://cdn.rawgit.com/stevekrouse/WoofJS/295004908a0ce07dd087954e7e7e79dab9227f89/index.html

Copy link
Owner

@stevekrouse stevekrouse left a comment

Choose a reason for hiding this comment

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

Looks great! Really excited about how thorough you were here.

One note I gave in multiple places was that I'd like you to put spaces after the // on comment lines. It's just a stylistic thing that's important to me.

description: "Beautify the selected code with proper indentation",
code: "Ctl-B"
}, ],
"Integrations": {
Copy link
Owner

Choose a reason for hiding this comment

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

Change the name of this #Keyboard class to #Integrations so the color styles apply to this button.

https://github.com/stevekrouse/WoofJS/blob/master/docs/docs.css#L232

docs/docs.css Outdated
@@ -393,6 +393,67 @@ h4 {
border-radius: 10px;
}

#Firebase {
Copy link
Owner

Choose a reason for hiding this comment

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

These colors don't seem to jive well with the color scheme of the rest of the docs buttons. Maybe use a color generator tool online to find ones that'd go well together and with the rest of the button colors. I imagine @nicolekelner could give good advice here too

docs/index.html Outdated
@@ -47,6 +47,9 @@
<script src="./docs.js"></script>
<script id="block-template" type="text/x-template">
<div class="block">
<div v-if="block.integrationDescription" class="alert" :class='block.integrationDescriptionClass' role='alert'>{{ block.integrationDescription }}<br v-if="block.integrationDocumentationLink"><br v-if="block.integrationDocumentationLink">
<a v-if="block.integrationDocumentationLink" target='_blank' class="btn btn-primary" :href="block.integrationDocumentationLink" role="button" style="width:100%;">Click Here For The API's Full Documentation</a>
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe the for this link could simply be "Full Documentation"

I try to avoid links that say "click here for".

I also probably want to avoid the phrase "API" in favor of the phrase "integration"

docs/index.html Outdated
"Integrations": {
Firebase: [
{
integrationDescription:"Firebase is Google's mobile platform that helps you with cloud storage.",
Copy link
Owner

Choose a reason for hiding this comment

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

I would describe Firebase as "database that helps you store cloud data" and drop the words "google" and "mobile" from the description

Copy link
Owner

Choose a reason for hiding this comment

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

I would also put Firebase towards the bottom of these integrations. It's hard to use. Definitely shouldn't be the first one.

docs/index.html Outdated
{html:"<hr style=\"height:7px;border:none;color:#e0e0e0;background-color:#e0e0e0;\"/>"},
{
description: "Import JavaScript code from an external URL. The second input happens after the code is imported.",
code: "importCodeURL('https://www.gstatic.com/firebasejs/3.4.0/firebase.js', () => {\n firebase.initializeApp({\/* credentials here */})\n})",
Copy link
Owner

Choose a reason for hiding this comment

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

This line of code doesn't really help you without the rest of the firebase functions discussed in the tutorial, so I would probably remove it from here.

You could include the importCodeURL either as its own integration or as a function in the More Blocks section, unrelated to firebase. That is, maybe demonstrate how to import another JS library other than firebase like the lodash library or something.

docs/index.html Outdated
},
{
description:"Here is a walk through of a program shows how much 10 GBP is currently worth in USD.",
code:'//First set up temporary text that will be replaced\nvar temp = new Text({\n text: "...", \n size: 60, \n color: "white", \n fontFamily: "arial",\n})\n//Then set your variables for the request\nvar base = "GBP"\n//Then set the url for your request\nvar url = "http://api.fixer.io/latest?base=" + base\n//Finaly call the getData() function\ngetData(url, data => {\n //Replace the temporary text with the converted amoung\n temp.text = Math.round( 10*data.rates.USD * 100) /100 + " USD"\n}) ',
Copy link
Owner

Choose a reason for hiding this comment

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

Spaces after // here.

Also could you format this expression like Math.round(10 * data.rates.USD * 100) / 100? Ditto for the second example below

docs/index.html Outdated
"Sunrise and Sunset Data":[
{
integrationDescription:"Use the Sunrise-Sunset API to get data about upcoming or past sunrises and sunsets.",
html:"<a target='_blank' href='http://www.latlong.net/'>Click here to find latitude and longitude values</a>",
Copy link
Owner

Choose a reason for hiding this comment

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

I would have it be more like, "You will need latitude and longitude values for this integration which you can find here."

docs/index.html Outdated
{html:"<hr style=\"height:7px;border:none;color:#e0e0e0;background-color:#e0e0e0;\"/>"},
{
description:"Here is a walk through of how to display today's sunrise time in NYC:",
code:'//First set up temporary text that will be replaced\nvar temp = new Text({\n text: "...", \n size: 60, \n color: "white", \nfontFamily: "arial",\n})\n//Then set your variables for the request\nvar lat = 40.7128\nvar lng = -74.0059 \n//Then set the url for your request\nvar url = "https://api.sunrise-sunset.org/json?lat=" + lat + "&lng=" + lng\n//Finaly call the getData() function\ngetData(url, data => {\n //Replace the temporary text\n temp.text = data.results.sunset\n}) ',
Copy link
Owner

Choose a reason for hiding this comment

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

I think this shows the sunset time

docs/index.html Outdated
tags:"trivia"
},
{
integrationDescription:"To generate your url visit the Trivia API Documentation.",
Copy link
Owner

Choose a reason for hiding this comment

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

I'd phrase this more like "You can customize the category and number of questions retrieved here."

docs/index.html Outdated
tags:"weather temperature"
},
{
description:"Here is a walk through of how to display the current wind speed and conditions in NYC:",
Copy link
Owner

Choose a reason for hiding this comment

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

I'd probably remove this extra example given how similar it is to the first one and that you have it below in the list

Copy link
Owner

@stevekrouse stevekrouse left a comment

Choose a reason for hiding this comment

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

I looked into the CORS issues you mentioned...

docs/index.html Outdated
// {
// description:"Here is a walk through of how to display a Chuck Norris joke from a specific category:",
// html:"<a target='_blank' href='https://api.chucknorris.io/jokes/categories'>Click here to see all the options for categories</a>",
// code:'//First set up temporary text that will be replaced\nvar temp = new Text({\n text: "...", \n size: 15, \n color: "black", \nfontFamily: "arial",\n})\n//Then set your variable for the request\nvar category = "sport"\n//Then set the url for your request\nvar url = "https://api.chucknorris.io/jokes/random?category=" + category\n//Finaly call the getData() function\ngetData(url, data => {\n //Replace the temporary text\n temp.text = data.value\n})',
Copy link
Owner

Choose a reason for hiding this comment

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

This worked for me. No CORS errors.

docs/index.html Outdated
// {html:"<hr style=\"height:7px;border:none;color:#e0e0e0;background-color:#e0e0e0;\"/>"},
// {
// description:"Here is a walk through of how to display the current stock price of Apple:",
// code:'//First set up temporary text that will be replaced\nvar temp = new Text({\n text: "...", \n size: 60, \n color: "white", \nfontFamily: "arial",\n})\n//Then set your variable for the request\nvar symbol = "AAPL"\n//Then set the url for your request\nvar url = "http://marketdata.websol.barchart.com/getQuote.json?key=1767804045c7c86775d9f86a36e06c55&symbols=" + symbol\n//Finaly call the getData() function\ngetData(url, data => {\n //Replace the temporary text\n temp.text = data.results.lastPrice\n})',
Copy link
Owner

Choose a reason for hiding this comment

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

We're going to need something called JSONP to make this work. This is tricky but not impossible.

Or we could use the library this guy created https://github.com/barchart/barchart-ondemand-client-js. This could be a decent demo of how to use importCodeURL

@jwass91
Copy link
Contributor Author

jwass91 commented Jul 5, 2017

I just pushed the revisions to the integrations. But there were a few things I wasn’t able to fix:

  • For the import code (which i moved back to more blocks) it didn’t seem to work for lodash so I left it importing firebase.
  • I wasn’t able to find the cloud data blocks on scratch to get a screenshot.
  • For now I left the market data commented while I work to implement JSONP or a different API but thought we might as well get the other integrations out for now.

@stevekrouse
Copy link
Owner

For the import code lodash thing, I was thinking it could look like the following which I was able to get working:

// import the lodash utility library
// https://lodash.com/docs/4.17.4
importCodeURL('https://cdn.jsdelivr.net/lodash/4.17.4/lodash.min.js', () => {
  _.flatten([1, [2, [3, [4]], 5]])
})

No worries about the cloud data screenshot.

I have a bit of a pet peeve against leaving in commented code that doesn't work. Here's how I'd prefer to solve this:

  1. Create a new branch finance-integration off of this integrations branch. There, uncomment the finance blocks. Push this new branch to github with a pull request. Explain that you still need to work on JSONP to get it to work.

  2. In this integrations branch, delete the finance blocks entirely and commit that. Then merge in this branch.

@jwass91
Copy link
Contributor Author

jwass91 commented Jul 6, 2017

Just took care of those last edits.

Now the finance integration here: #430

@stevekrouse stevekrouse merged commit 2f2d485 into master Jul 6, 2017
@stevekrouse stevekrouse deleted the integrations branch July 6, 2017 20:33
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