Skip to content

Don't try to fetch map data unless #map is present #631

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 1 commit into from
Jan 30, 2018

Conversation

denmch
Copy link
Contributor

@denmch denmch commented Jan 6, 2018

Closes issue #627, which raised the issue of a console error that's triggered by this file's attempt to fetch google maps data on pages without the requisite #map element. This is a simple fix that checks to see whether #map is present before proceeding with the .getJSON AJAX function. Only the /intro-to-github/ page contains the map, so the console error appears on every other page.

Issue github#627 raised the issue of a console error that's triggered by this file's attempt to fetch google maps data on pages without the requisite `#map` element. This is a simple fix that checks to see whether `#map` is present before proceeding with the `.getJSON` AJAX function.
@brianamarie
Copy link
Contributor

Thank you, @denmch! Since my Javascript is a little rusty, @JasonEtco do you have bandwidth to put 👀 on this?

@denmch
Copy link
Contributor Author

denmch commented Jan 8, 2018

No problem, @brianamarie. It looks like a bigger change than it is because of whitespace changes, but the only addition is a jQuery conditional (if ($("#map").length)) to see if #map exists and to prevent looking for data where there isn't any. It seemed like a simple way to prevent the console error without fussing too much over things like loading the JS only on the page needed (which would touch more things and is something that could still be revisited later if anyone deems it an issue).

@brianamarie
Copy link
Contributor

cc @hectorsector for 👀

@brianamarie
Copy link
Contributor

@stoe Now that you're back around, would you have a few minutes to look at this JS?

@brianamarie
Copy link
Contributor

@stoe Thank you for the review and @denmch Thank you for your contributions!!!

I was a little hesitant because when I tried to build with gh-pages, I got an error. When I built locally, it works beautifully. Thanks! ✨

@brianamarie brianamarie merged commit c293d83 into github:master Jan 30, 2018
@brianamarie
Copy link
Contributor

Note, this won't be shown live until the submodule is updated.

@denmch
Copy link
Contributor Author

denmch commented Jan 30, 2018

I'm glad it worked! I wouldn't have been surprised if I'd messed something up.

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.

3 participants