Skip to content

Commit

Permalink
Fix full page map keyboard on android, and set current center correct…
Browse files Browse the repository at this point in the history
…ly (#1101)

This PR fixes a bug where, on Android, it was not possible to run searches on the full page map, because
clicking into the keyboard would trigger a "searchOnMapMove", which would cause new search results to come in
and cause the keyboard to collapse itself.

The first issue is that, the android keyboard shifts all content on the page up, unlike the iOS keyboard, which does
not affect page content. This would cause the center of the map to move, which would cause searchOnMapMove
to be triggered. This was dealt with by fixing the height of the mapWrapper on page load for android devices.

The second issue was that the VerticalFullPageMapOrchestrator would ALWAYS have the incorrect map center.
This is because it would grab the center of the map before the map had actually loaded, resulting in it thinking the
center of the map was always the `this.defaultCenter` value, when it is virtually never that. This would make
it so that even with the first issue being patched, the keyboard would still always collapse itself the first time
you clicked into it, since the VerticalFullPageMapOrchestrator would not have the correct center until after that.
To patch this I added a new loadHandler to the map, and update the center in that handler.

J=TECHOPS-6738
TEST=manual

test on browserstack android chrome that I can click into the searchbar on the full page map, for both google and mapbox, and no search will be triggered and the keyboard will not hide itself and refuse to be typed into

test that locally on desktop the page looks fine on both google and mapbox
  • Loading branch information
oshi97 committed Oct 4, 2022
1 parent 5933c74 commit 4ad8d80
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 4 deletions.
3 changes: 2 additions & 1 deletion static/js/constants/storage-keys.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ const StorageKeys = {
LOCATOR_SELECTED_RESULT: 'locator-selected-result',
LOCATOR_NUM_CONCURRENT_SEARCH_THIS_AREA_CALLS: 'locator-num-concurrent-search-this-area-calls',
LOCATOR_MAP_PROPERTIES: 'locator-map-properties',
LOCATOR_CARD_FOCUS: 'locator-card-focus'
LOCATOR_CARD_FOCUS: 'locator-card-focus',
MAP_LOADED: 'map-loaded'
};

export default StorageKeys;
28 changes: 28 additions & 0 deletions static/js/theme-map/Maps/Map.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class MapOptions {
this.zoomChangedHandler = () => {};
this.zoomEndHandler = () => {};
this.canvasClickHandler = () => {};
this.loadHandler = () => {};
this.provider = null;
this.providerOptions = {};
this.singlePinZoom = 14;
Expand Down Expand Up @@ -136,6 +137,22 @@ class MapOptions {
return this;
}

/**
* @typedef Map~handler
* @function
*/

/**
* @param {Map~handler} handler
* @returns {MapOptions}
*/
withLoadHandler(handler) {
assertType(handler, Type.FUNCTION);

this.handler = handler;
return this;
}

/**
* @typedef Map~panStartHandler
* @function
Expand Down Expand Up @@ -310,6 +327,7 @@ class Map {
this.setZoomChangedHandler(options.zoomChangedHandler);
this.setZoomEndHandler(options.zoomEndHandler);
this.setCanvasClickHandler(options.canvasClickHandler);
this.setLoadHandler(options.loadHandler);

// Remove all child elements of wrapper
while (this._wrapper.firstChild) {
Expand All @@ -328,6 +346,7 @@ class Map {
.withPanStartHandler(() => this.panStartHandler())
.withProviderOptions(options.providerOptions)
.withLocale(options.locale)
.withLoadHandler(() => this._loadHandler())
.build();

this.setZoomCenter(this._defaultZoom, this._defaultCenter);
Expand Down Expand Up @@ -753,6 +772,15 @@ class Map {
this._canvasClickHandler = canvasClickHandler;
}

/**
* @param {Map~loadHandler} handler
*/
setLoadHandler(handler) {
assertType(handler, Type.FUNCTION);

this._loadHandler = handler;
}

/**
* @param {number} zoom
* @param {boolean} [animated=false] Whether to transition smoothly to the new zoom
Expand Down
13 changes: 13 additions & 0 deletions static/js/theme-map/Maps/ProviderMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class ProviderMapOptions {
this.zoomChangedHandler = () => {};
this.zoomEndHandler = () => {};
this.canvasClickHandler = () => {};
this.loadHandler = () => {};
this.providerOptions = {};
this.locale = 'en';
}
Expand All @@ -44,6 +45,17 @@ class ProviderMapOptions {
return this;
}

/**
* @typedef ProviderMap~loadHandler
* @function
*/

withLoadHandler(handler) {
assertType(handler, Type.FUNCTION);
this.loadHandler = handler;
return this;
}

/**
* @typedef ProviderMap~panHandler
* @function
Expand Down Expand Up @@ -182,6 +194,7 @@ class ProviderMap {
this._zoomChangedHandler = options.zoomChangedHandler;
this._zoomEndHandler = options.zoomEndHandler;
this._canvasClickHandler = options.canvasClickHandler;
this._loadHandler = options.loadHandler;
}

/**
Expand Down
3 changes: 3 additions & 0 deletions static/js/theme-map/Maps/Providers/Google.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ class GoogleMap extends ProviderMap {
google.maps.event.addListener(this.map, 'click', () => {
this._canvasClickHandler();
});
google.maps.event.addListenerOnce(this.map, 'tilesloaded', () => {
this._loadHandler()
})
}

getCenter() {
Expand Down
3 changes: 3 additions & 0 deletions static/js/theme-map/Maps/Providers/Mapbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ class MapboxMap extends ProviderMap {
this._canvasClickHandler();
}
});
this.map.on('load', () => {
this._loadHandler();
})
}

getCenter() {
Expand Down
3 changes: 3 additions & 0 deletions static/js/theme-map/ThemeMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ class ThemeMap extends ANSWERS.Component {
this.config.zoomEndListener(this.map.getZoom(), zoomTrigger);
});
map.setCanvasClickHandler(() => this.config.canvasClickListener());
map.setLoadHandler(() => {
this.core.storage.set(StorageKeys.MAP_LOADED, true)
})
});

const mapRenderTargetOptions = new MapRenderTargetOptions()
Expand Down
48 changes: 46 additions & 2 deletions static/js/theme-map/VerticalFullPageMapOrchestrator.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,20 @@ class VerticalFullPageMapOrchestrator extends ANSWERS.Component {
}

onCreate () {
this.core.storage.registerListener({
eventType: 'update',
storageKey: StorageKeys.MAP_LOADED,
callback: () => {
this.updateMostRecentSearchState()
}
})

this.core.storage.registerListener({
eventType: 'update',
storageKey: StorageKeys.VERTICAL_RESULTS,
callback: (data) => this.setState(data)
callback: (data) => {
this.setState(data)
}
});

this.core.storage.registerListener({
Expand All @@ -193,9 +203,43 @@ class VerticalFullPageMapOrchestrator extends ANSWERS.Component {
searchThisAreaButtonEl.addEventListener('click', (e) => {
this.searchThisArea();
});

this.setupMobileBreakpointListener();
this.addMapComponent();
this.setFixedHeightsOnAndroid();
}

/**
* On Android browsers, opening up the keyboard will shift the contents of the entire page up,
* moving the map center, and thereby causing a searchOnMapMove to be triggered.
* The search response would then cause the page to update,
* and close the keyboard, making it impossible to actually type anything into the searchbar.
*
* Setting a fixed height on elements like .Answers-mapWrapper prevents the keyboard from shifting the content
* of the page.
*/
setFixedHeightsOnAndroid() {
if (!this.isMobile() || !/Android/i.test(navigator.userAgent)) {
return;
}

setFixedHeight('.Answers-mapWrapper')

function getSingleElement(selector) {
const els = document.querySelectorAll(selector);
if (els.length === 0) {
console.error(`No ${selector} found, unable to set fixed height for the full page map.`);
} else if (els.length > 1) {
console.error(
`Multiple elements for ${selector} found, expected only 1, not setting fixed height for the full page map.`);
} else {
return els[0];
}
}

function setFixedHeight(selector) {
const el = getSingleElement(selector)
el.style.height = `${el.scrollHeight}px`
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion test-site/script/on-ready.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@
ANSWERS.setGeolocation(38.8955, -77.0699)
ANSWERS.setGeolocation(38.8955, -77.0699);

0 comments on commit 4ad8d80

Please sign in to comment.