From 3cdcf6b295eb77bfee6c0e014628846891d3babc Mon Sep 17 00:00:00 2001 From: Vaillant Samuel Date: Mon, 17 Sep 2018 09:44:17 +0200 Subject: [PATCH] fix(widget): never allow postion & IP at the same time --- .../__snapshots__/widget.test.js.snap | 2 +- src/instantsearch/widget.js | 40 +++++- src/instantsearch/widget.test.js | 125 ++++++++++++------ 3 files changed, 122 insertions(+), 45 deletions(-) diff --git a/src/instantsearch/__snapshots__/widget.test.js.snap b/src/instantsearch/__snapshots__/widget.test.js.snap index 1fe3bcb6a..e819671ec 100644 --- a/src/instantsearch/__snapshots__/widget.test.js.snap +++ b/src/instantsearch/__snapshots__/widget.test.js.snap @@ -7,7 +7,7 @@ SearchParameters { "analytics": undefined, "analyticsTags": undefined, "aroundLatLng": "123,123", - "aroundLatLngViaIP": undefined, + "aroundLatLngViaIP": false, "aroundPrecision": undefined, "aroundRadius": undefined, "attributesToHighlight": undefined, diff --git a/src/instantsearch/widget.js b/src/instantsearch/widget.js index 3c66ce134..23d0d2800 100644 --- a/src/instantsearch/widget.js +++ b/src/instantsearch/widget.js @@ -13,21 +13,31 @@ class AlgoliaPlacesWidget { this.placesAutocomplete = places(this.placesOptions); this.query = ''; + this.initialLatLngViaIP = null; } getConfiguration() { const configuration = {}; if (this.defaultPosition) { - configuration.aroundLatLng = this.defaultPosition; configuration.insideBoundingBox = undefined; + configuration.aroundLatLngViaIP = false; + configuration.aroundLatLng = this.defaultPosition; } return configuration; } init({ helper }) { - const viaIPFlag = helper.getQueryParameter('aroundLatLngViaIP'); + // Get the initial value only when it's not already set via URLSync + // see: getWidgetSearchParameters + if (this.initialLatLngViaIP === null) { + // The value is retrieved in the `init` rather than `getConfiguration` + // because the widget that set `aroundLatLngViaIP` might be registered + // after this one. We wait until we have the full configuration to save + // the initial value. + this.initialLatLngViaIP = helper.getQueryParameter('aroundLatLngViaIP'); + } this.placesAutocomplete.on('change', opts => { const { @@ -38,6 +48,7 @@ class AlgoliaPlacesWidget { } = opts; this.query = value; + helper .setQueryParameter('insideBoundingBox') .setQueryParameter('aroundLatLngViaIP', false) @@ -47,11 +58,20 @@ class AlgoliaPlacesWidget { this.placesAutocomplete.on('clear', () => { this.query = ''; - helper - .setQueryParameter('insideBoundingBox') - .setQueryParameter('aroundLatLngViaIP', viaIPFlag) - .setQueryParameter('aroundLatLng', this.defaultPosition) - .search(); + + helper.setQueryParameter('insideBoundingBox'); + + if (this.defaultPosition) { + helper + .setQueryParameter('aroundLatLngViaIP', false) + .setQueryParameter('aroundLatLng', this.defaultPosition); + } else { + helper + .setQueryParameter('aroundLatLngViaIP', this.initialLatLngViaIP) + .setQueryParameter('aroundLatLng'); + } + + helper.search(); }); } @@ -59,17 +79,23 @@ class AlgoliaPlacesWidget { if (!uiState.places) { this.placesAutocomplete.setVal(''); this.placesAutocomplete.close(); + return searchParameters; } const { query, position } = uiState.places; this.query = query; + this.initialLatLngViaIP = searchParameters.getQueryParameter( + 'aroundLatLngViaIP' + ); + this.placesAutocomplete.setVal(query || ''); this.placesAutocomplete.close(); return searchParameters .setQueryParameter('insideBoundingBox') + .setQueryParameter('aroundLatLngViaIP', false) .setQueryParameter('aroundLatLng', position); } diff --git a/src/instantsearch/widget.test.js b/src/instantsearch/widget.test.js index 58a6ebda5..87880de6c 100644 --- a/src/instantsearch/widget.test.js +++ b/src/instantsearch/widget.test.js @@ -79,6 +79,7 @@ describe('instantsearch widget', () => { expect(afterConfiguration).toEqual({ insideBoundingBox: undefined, + aroundLatLngViaIP: false, aroundLatLng: '1,1', }); }); @@ -94,6 +95,7 @@ describe('instantsearch widget', () => { expect(afterConfiguration).toEqual({ insideBoundingBox: undefined, + aroundLatLngViaIP: false, aroundLatLng: '1,1', }); }); @@ -137,6 +139,7 @@ describe('instantsearch widget', () => { const widget = algoliaPlacesWidget(defaultOptions); helper.setQueryParameter('aroundLatLngViaIP', true); + widget.init({ helper }); const eventName = places.__instance.on.mock.calls[0][0]; @@ -154,11 +157,13 @@ describe('instantsearch widget', () => { }); }); - it('configures aroundLatLng on clear event', () => { + it('configures aroundLatLng with a default position on clear event', () => { const client = createFakeClient(); const helper = createFakekHelper(client); const widget = algoliaPlacesWidget({ defaultPosition: [2, 2] }); + helper.setQueryParameter('aroundLatLngViaIP', true); + widget.init({ helper }); const eventName = places.__instance.on.mock.calls[1][0]; @@ -171,10 +176,56 @@ describe('instantsearch widget', () => { expect(helper.search).toBeCalled(); expect(helper.getState()).toMatchObject({ insideBoundingBox: undefined, + aroundLatLngViaIP: false, aroundLatLng: '2,2', }); }); + it('restores aroundLatLngViaIP without a default position on clear event', () => { + const client = createFakeClient(); + const helper = createFakekHelper(client); + const widget = algoliaPlacesWidget(); + + helper.setQueryParameter('aroundLatLngViaIP', true); + + widget.init({ helper }); + + const changeEventName = places.__instance.on.mock.calls[0][0]; + const changeEventListener = places.__instance.on.mock.calls[0][1]; + + expect(changeEventName).toEqual('change'); + + changeEventListener({ + suggestion: { + latlng: { + lat: '123', + lng: '456', + }, + }, + }); + + expect(helper.search).toBeCalled(); + expect(helper.getState()).toMatchObject({ + insideBoundingBox: undefined, + aroundLatLng: '123,456', + aroundLatLngViaIP: false, + }); + + const clearEventName = places.__instance.on.mock.calls[1][0]; + const clearEventListener = places.__instance.on.mock.calls[1][1]; + + expect(clearEventName).toEqual('clear'); + + clearEventListener(); + + expect(helper.search).toBeCalled(); + expect(helper.getState()).toMatchObject({ + insideBoundingBox: undefined, + aroundLatLng: undefined, + aroundLatLngViaIP: true, + }); + }); + describe('routing', () => { const getInitializedWidget = ( widgetOptions = { defaultPosition: [2, 2] } @@ -182,11 +233,47 @@ describe('instantsearch widget', () => { const client = createFakeClient(); const helper = createFakekHelper(client); const widget = algoliaPlacesWidget(widgetOptions); + widget.init({ helper, state: helper.state }); return [widget, helper]; }; + it('restores aroundLatLngViaIP on clear event', () => { + const client = createFakeClient(); + const helper = createFakekHelper(client); + const widget = algoliaPlacesWidget(); + + // Simulate the fact that a widget set `aroundLatLngViaIP` from the URLSync + const searchParametersBefore = SearchParameters.make({ + aroundLatLngViaIP: true, + }); + + const uiState = { + places: { + position: '123,123', + query: 'Paris', + }, + }; + + widget.getWidgetSearchParameters(searchParametersBefore, { + uiState, + }); + + widget.init({ helper }); + + const clearEventListener = places.__instance.on.mock.calls[1][1]; + + clearEventListener(); + + expect(helper.search).toBeCalled(); + expect(helper.getState()).toMatchObject({ + insideBoundingBox: undefined, + aroundLatLng: undefined, + aroundLatLngViaIP: true, + }); + }); + describe('getWidgetState', () => { test('should give back the object unmodified if the default value is selected', () => { const [widget, helper] = getInitializedWidget(); @@ -394,40 +481,4 @@ describe('instantsearch widget', () => { }); }); }); - - it('restores aroundLatLngVia:true on clear event', () => { - const client = createFakeClient(); - const helper = createFakekHelper(client); - const widget = algoliaPlacesWidget({ defaultPosition: [2, 2] }); - - helper.setQueryParameter('aroundLatLngViaIP', true); - widget.init({ helper }); - const changeEventName = places.__instance.on.mock.calls[0][0]; - const changeEventListener = places.__instance.on.mock.calls[0][1]; - - expect(changeEventName).toEqual('change'); - - changeEventListener({ suggestion: { latlng: { lat: '123', lng: '456' } } }); - - expect(helper.search).toBeCalled(); - expect(helper.getState()).toMatchObject({ - insideBoundingBox: undefined, - aroundLatLng: '123,456', - aroundLatLngViaIP: false, - }); - - const clearEventName = places.__instance.on.mock.calls[1][0]; - const clearEventListener = places.__instance.on.mock.calls[1][1]; - - expect(clearEventName).toEqual('clear'); - - clearEventListener(); - - expect(helper.search).toBeCalled(); - expect(helper.getState()).toMatchObject({ - insideBoundingBox: undefined, - aroundLatLng: '2,2', - aroundLatLngViaIP: true, - }); - }); });