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.

Sign up for GitHub

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

Check if "google" is defined #269

Merged
merged 4 commits into from
Feb 6, 2019
Merged

Check if "google" is defined #269

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
93c0751
Update PublicLab.MapModule.js
rexagod Feb 1, 2019
519dd88
Merge branch 'master' into google-patch
jywarren Feb 6, 2019
5b1f93f
Merge branch 'master' into google-patch
rexagod Feb 6, 2019
b066e64
Update PublicLab.MapModule.js
rexagod Feb 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/modules/PublicLab.MapModule.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@ module.exports = PublicLab.MapModule = PublicLab.Module.extend({
}

_module.blurredLocation = new BlurredLocation(options) ;

_module.blurredLocation.panMapToGeocodedLocation("placenameInput") ;

//check if "google" is defined PLOTS2#4717
google
Copy link
Member

Choose a reason for hiding this comment

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

Does this have to be window.hasOwnProperty('google')?

Copy link
Member Author

@rexagod rexagod Feb 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I tested this snippet @jywarren, there's no need to explicitly check the window object, since google gets binded to it when we include the scripts, but I can go ahead and add this to make the code a bit more semantic. What do you say?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, really? Just running this locally i get:

google ? alert('hi') : alert('bye')
> Uncaught ReferenceError: google is not defined

Sorry, just trying to be extra sure -- and perhaps I misunderstand how this will be run, so happy to learn!

Copy link
Member Author

@rexagod rexagod Feb 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@jywarren I was unable to reproduce this for some reason, google always shows up defined on my local, but binding this to window would be a good step here just to be sure! 🤔 I'll make the changes.

peek 2019-02-06 23-57

Copy link
Member

Choose a reason for hiding this comment

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

is it possible it is actually defined there? Can you try it with, say, a made- up variable name? I think we're watching for when the google library is not included, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like both approaches solved the issue. 😄

screenshot from 2019-02-07 01-26-12

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh! Didn't see your comment there since we both posted at the same time. Looking into this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you try it with, say, a made- up variable name?

screenshot from 2019-02-07 01-34-12

? _module.blurredLocation.panMapToGeocodedLocation("placenameInput")
: console.log("`google` is not defined! PublicLab.MapModule.js#28");

_module.blurredLocation.setBlurred(false) ;

_module.value = function(){
Expand Down