Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

docs(guide/Conceptual Overview): fix external api example #15336

Merged
merged 2 commits into from
Oct 31, 2016

Conversation

Narretz
Copy link
Contributor

@Narretz Narretz commented Oct 29, 2016

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
docs

What is the current behavior? (You can also link to an open issue here)
Example is broken in the snapshot because of changes to JSONP

Other information:

The example will still be broken when opened in a plnkr, because they use the stable version instead of the snapshot.

Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

BTW, I think we could just use .get() instead of .jsonp().

}]);
</file>
<file name="finance3.js">
angular.module('finance3', [])
.factory('currencyConverter', ['$http', function($http) {
var YAHOO_FINANCE_URL_PATTERN =
'//query.yahooapis.com/v1/public/yql?q=select * from ' +
'https://query.yahooapis.com/v1/public/yql?q=select * from ' +
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary? This won't work if the plnkr is loaded ovr http: iirc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The plnkrs are loaded over https. I had to add it, otherwise it didn't match.

@Narretz
Copy link
Contributor Author

Narretz commented Oct 29, 2016

True, the api sets Access-Control-Allow-Origin:"*". I'll make that change.

@gkalpak
Copy link
Member

gkalpak commented Oct 30, 2016

LGTM

@Narretz Narretz merged commit e77f717 into angular:master Oct 31, 2016
@Narretz Narretz deleted the docs-concepts-jsonp branch October 31, 2016 09:29
ellimist pushed a commit to ellimist/angular.js that referenced this pull request Mar 15, 2017
In 1.6, urls accessed with jsonp must be whitelisted via sce. 
However, the yahoo finance api used in the example allows CORS
access via Access-Control-Allow-Origin:"*", so we can simply use
`$http.get` instead.

Closes angular#15336
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants