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.

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

Fix locator bundle race condition #786

Merged
merged 13 commits into from
May 24, 2021

Conversation

cea2aj
Copy link
Member

@cea2aj cea2aj commented May 20, 2021

Fix the race condition which occurs when the search query loads before the locator bundle

This fix ensures that the locator bundle executes after Answers loads, but before Answers initializes. The race condition issue was first found on Theme 1.20.1, and it caused the cards to break when the page loaded. I put a fix up in #739, which stopped the page from breaking, but it did not fix the underlying cause of the race condition. This PR fixes the root problem by making the execution order of the javascript consistent.

The race condition lead to an issue where two queries were ran on page load. This became a problem in a techops (Zendesk 405748) because the page would load and return no results while being zoomed into Kansas. This PR prevents the double queries from being sent if the locator bundle takes a while to load. There still may be some situations where two queries are sent on page load, but those issues are out of scope of this particular issue. I also zoomed out the map by default so that if no default initial search is supplied, the map will show the entire US rather than be zoomed into Kansas.

J=SLAP-1329
TEST=manual

Test both Google Maps and Mapbox. Use fiddler to artificially delay the locator bundle. Test iframe and non-iframe experiences.

script/core.hbs Outdated Show resolved Hide resolved
script/core.hbs Outdated Show resolved Hide resolved
script/core.hbs Outdated Show resolved Hide resolved
static/js/page-specific-scripts-loaded.js Outdated Show resolved Hide resolved
script/core.hbs Outdated Show resolved Hide resolved
@cea2aj cea2aj requested a review from tmeyer2115 May 24, 2021 16:55
@cea2aj cea2aj requested a review from oshi97 May 24, 2021 19:02
Copy link
Contributor

@oshi97 oshi97 left a comment

Choose a reason for hiding this comment

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

lgtm!

@cea2aj cea2aj merged commit 4fb6398 into hotfix/v1.21.1 May 24, 2021
@cea2aj cea2aj deleted the dev/locator-bundle-race-condtion branch May 24, 2021 20:01
@cea2aj cea2aj mentioned this pull request Jun 7, 2021
cea2aj added a commit that referenced this pull request Jun 7, 2021
### Bugfixes
- Fix a race condition on the full page map (#786)
- Fixed a bug on the full page map where NLP filters in the search query would not work properly after a 'Search this Area' search was performed (#819)
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