Skip to content

Commit

Permalink
Fix #3138 Repeated get feature info requests cause visual glitch if w…
Browse files Browse the repository at this point in the history
…idgets are on map (#3139)
  • Loading branch information
allyoucanmap committed Aug 3, 2018
1 parent ed8dd13 commit c483be2
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 21 deletions.
7 changes: 7 additions & 0 deletions web/client/actions/mapInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const CLEAR_WARNING = 'CLEAR_WARNING';
const FEATURE_INFO_CLICK = 'FEATURE_INFO_CLICK';
const TOGGLE_MAPINFO_STATE = 'TOGGLE_MAPINFO_STATE';
const UPDATE_CENTER_TO_MARKER = 'UPDATE_CENTER_TO_MARKER';
const CLOSE_IDENTIFY = 'IDENTIFY:CLOSE_IDENTIFY';

/**
* Private
Expand Down Expand Up @@ -208,6 +209,10 @@ function updateCenterToMarker(status) {
};
}

const closeIdentify = () => ({
type: CLOSE_IDENTIFY
});

module.exports = {
ERROR_FEATURE_INFO,
EXCEPTIONS_FEATURE_INFO,
Expand All @@ -226,6 +231,8 @@ module.exports = {
FEATURE_INFO_CLICK,
TOGGLE_MAPINFO_STATE,
UPDATE_CENTER_TO_MARKER,
CLOSE_IDENTIFY,
closeIdentify,
getFeatureInfo,
changeMapInfoState,
newMapInfoRequest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,4 +369,31 @@ describe("test identify enhancers", () => {
expect(spySetIndex.calls.length).toEqual(0);
});

it("test identifyLifecycle on close", () => {
const Component = identifyLifecycle(({onClose = () => {}}) => <div id="test-component" onClick={() => onClose()}></div>);
const testHandlers = {
closeIdentify: () => {},
purgeResults: () => {},
hideMarker: () => {}
};
const spyCloseIdentify = expect.spyOn(testHandlers, 'closeIdentify');
const spyPurgeResults = expect.spyOn(testHandlers, 'purgeResults');
const spyHideMarker = expect.spyOn(testHandlers, 'hideMarker');
ReactDOM.render(
<Component
enabled
responses={[{}]}
closeIdentify={testHandlers.closeIdentify}
purgeResults={testHandlers.purgeResults}
hideMarker={testHandlers.hideMarker}/>,
document.getElementById("container")
);

const testComponent = document.getElementById('test-component');
TestUtils.Simulate.click(testComponent);
expect(spyCloseIdentify).toHaveBeenCalled();
expect(spyPurgeResults).toHaveBeenCalled();
expect(spyHideMarker).toHaveBeenCalled();
});

});
3 changes: 2 additions & 1 deletion web/client/components/data/identify/enhancers/identify.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@ const identifyHandlers = withHandlers({
}
return false;
},
onClose: ({hideMarker, purgeResults}) => () => {
onClose: ({hideMarker = () => {}, purgeResults = () => {}, closeIdentify = () => {}}) => () => {
hideMarker();
purgeResults();
closeIdentify();
}
});

Expand Down
17 changes: 17 additions & 0 deletions web/client/epics/__tests__/maplayout-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const expect = require('expect');

const {toggleControl} = require('../../actions/controls');
const {UPDATE_MAP_LAYOUT} = require('../../actions/maplayout');
const {closeIdentify} = require('../../actions/mapInfo');
const {updateMapLayoutEpic} = require('../maplayout');
const {testEpic} = require('./epicTestUtils');

Expand Down Expand Up @@ -53,4 +54,20 @@ describe('map layout epics', () => {
const state = {mode: 'embedded', controls: { drawer: {enabled: true}}};
testEpic(updateMapLayoutEpic, 1, toggleControl("queryPanel"), epicResult, state);
});

it('tests on close identify', (done) => {
const epicResult = actions => {
try {
expect(actions.length).toBe(1);
actions.map((action) => {
expect(action.type).toBe(UPDATE_MAP_LAYOUT);
});
} catch(e) {
done(e);
}
done();
};
const state = {};
testEpic(updateMapLayoutEpic, 1, closeIdentify(), epicResult, state);
});
});
40 changes: 21 additions & 19 deletions web/client/epics/maplayout.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const {updateMapLayout} = require('../actions/maplayout');
const {TOGGLE_CONTROL, SET_CONTROL_PROPERTY} = require('../actions/controls');
const {MAP_CONFIG_LOADED} = require('../actions/config');
const {SIZE_CHANGE, CLOSE_FEATURE_GRID, OPEN_FEATURE_GRID} = require('../actions/featuregrid');
const {PURGE_MAPINFO_RESULTS, ERROR_FEATURE_INFO} = require('../actions/mapInfo');
const {CLOSE_IDENTIFY, ERROR_FEATURE_INFO, TOGGLE_MAPINFO_STATE, LOAD_FEATURE_INFO, EXCEPTIONS_FEATURE_INFO} = require('../actions/mapInfo');
const {SHOW_SETTINGS, HIDE_SETTINGS} = require('../actions/layers');
const {mapInfoRequestsSelector} = require('../selectors/mapinfo');

Expand All @@ -24,19 +24,21 @@ const {head, get} = require('lodash');
const {isFeatureGridOpen, getDockSize} = require('../selectors/featuregrid');

/**
* Gets `MAP_CONFIG_LOADED`, `SIZE_CHANGE`, `CLOSE_FEATURE_GRID`, `OPEN_FEATURE_GRID`, `PURGE_MAPINFO_RESULTS`, `TOGGLE_CONTROL`, `SET_CONTROL_PROPERTY` events.
* Gets `MAP_CONFIG_LOADED`, `SIZE_CHANGE`, `CLOSE_FEATURE_GRID`, `OPEN_FEATURE_GRID`, `CLOSE_IDENTIFY`, `LOAD_FEATURE_INFO`, `TOGGLE_MAPINFO_STATE`, `TOGGLE_CONTROL`, `SET_CONTROL_PROPERTY` events.
* Configures a map layout based on state of panels.
* @param {external:Observable} action$ manages `MAP_CONFIG_LOADED`, `SIZE_CHANGE`, `CLOSE_FEATURE_GRID`, `OPEN_FEATURE_GRID`, `PURGE_MAPINFO_RESULTS`, `TOGGLE_CONTROL`, `SET_CONTROL_PROPERTY`.
* @param {external:Observable} action$ manages `MAP_CONFIG_LOADED`, `SIZE_CHANGE`, `CLOSE_FEATURE_GRID`, `OPEN_FEATURE_GRID`, `CLOSE_IDENTIFY`, `LOAD_FEATURE_INFO`, `TOGGLE_MAPINFO_STATE`, `TOGGLE_CONTROL`, `SET_CONTROL_PROPERTY`.
* @memberof epics.mapLayout
* @return {external:Observable} emitting {@link #actions.map.updateMapLayout} action
*/

const updateMapLayoutEpic = (action$, store) =>
action$.ofType(MAP_CONFIG_LOADED, SIZE_CHANGE, CLOSE_FEATURE_GRID, OPEN_FEATURE_GRID, PURGE_MAPINFO_RESULTS, TOGGLE_CONTROL, SET_CONTROL_PROPERTY, SHOW_SETTINGS, HIDE_SETTINGS, ERROR_FEATURE_INFO)
action$.ofType(MAP_CONFIG_LOADED, SIZE_CHANGE, CLOSE_FEATURE_GRID, OPEN_FEATURE_GRID, CLOSE_IDENTIFY, TOGGLE_MAPINFO_STATE, LOAD_FEATURE_INFO, EXCEPTIONS_FEATURE_INFO, TOGGLE_CONTROL, SET_CONTROL_PROPERTY, SHOW_SETTINGS, HIDE_SETTINGS, ERROR_FEATURE_INFO)
.switchMap(() => {

if (get(store.getState(), "browser.mobile")) {
const bottom = mapInfoRequestsSelector(store.getState()).length > 0 ? {bottom: '50%'} : {bottom: undefined};
const state = store.getState();

if (get(state, "browser.mobile")) {
const bottom = mapInfoRequestsSelector(state).length > 0 ? {bottom: '50%'} : {bottom: undefined};
const boundingMapRect = {
...bottom
};
Expand All @@ -47,9 +49,9 @@ const updateMapLayoutEpic = (action$, store) =>

const mapLayout = {left: {sm: 300, md: 500, lg: 600}, right: {md: 658}, bottom: {sm: 30}};

if (get(store.getState(), "mode") === 'embedded') {
if (get(state, "mode") === 'embedded') {
const height = {height: 'calc(100% - ' + mapLayout.bottom.sm + 'px)'};
const bottom = mapInfoRequestsSelector(store.getState()).length > 0 ? {bottom: '50%'} : {bottom: undefined};
const bottom = mapInfoRequestsSelector(state).length > 0 ? {bottom: '50%'} : {bottom: undefined};
const boundingMapRect = {
...bottom
};
Expand All @@ -60,23 +62,23 @@ const updateMapLayoutEpic = (action$, store) =>
}

const leftPanels = head([
get(store.getState(), "controls.queryPanel.enabled") && {left: mapLayout.left.lg} || null,
get(store.getState(), "controls.widgetBuilder.enabled") && {left: mapLayout.left.md} || null,
get(store.getState(), "layers.settings.expanded") && {left: mapLayout.left.md} || null,
get(store.getState(), "controls.drawer.enabled") && {left: mapLayout.left.sm} || null
get(state, "controls.queryPanel.enabled") && {left: mapLayout.left.lg} || null,
get(state, "controls.widgetBuilder.enabled") && {left: mapLayout.left.md} || null,
get(state, "layers.settings.expanded") && {left: mapLayout.left.md} || null,
get(state, "controls.drawer.enabled") && {left: mapLayout.left.sm} || null
].filter(panel => panel)) || {left: 0};

const rightPanels = head([
get(store.getState(), "controls.details.enabled") && {right: mapLayout.right.md} || null,
get(store.getState(), "controls.annotations.enabled") && {right: mapLayout.right.md} || null,
get(store.getState(), "controls.metadataexplorer.enabled") && {right: mapLayout.right.md} || null,
mapInfoRequestsSelector(store.getState()).length > 0 && {right: mapLayout.right.md} || null
get(state, "controls.details.enabled") && {right: mapLayout.right.md} || null,
get(state, "controls.annotations.enabled") && {right: mapLayout.right.md} || null,
get(state, "controls.metadataexplorer.enabled") && {right: mapLayout.right.md} || null,
get(state, "mapInfo.enabled") && mapInfoRequestsSelector(state).length > 0 && {right: mapLayout.right.md} || null
].filter(panel => panel)) || {right: 0};

const dockSize = getDockSize(store.getState()) * 100;
const bottom = isFeatureGridOpen(store.getState()) && {bottom: dockSize + '%', dockSize} || {bottom: mapLayout.bottom.sm};
const dockSize = getDockSize(state) * 100;
const bottom = isFeatureGridOpen(state) && {bottom: dockSize + '%', dockSize} || {bottom: mapLayout.bottom.sm};

const transform = isFeatureGridOpen(store.getState()) && {transform: 'translate(0, -' + mapLayout.bottom.sm + 'px)'} || {transform: 'none'};
const transform = isFeatureGridOpen(state) && {transform: 'translate(0, -' + mapLayout.bottom.sm + 'px)'} || {transform: 'none'};
const height = {height: 'calc(100% - ' + mapLayout.bottom.sm + 'px)'};

const boundingMapRect = {
Expand Down
3 changes: 2 additions & 1 deletion web/client/plugins/Identify.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const {on} = require('../actions/controls');
const {getFeatureInfo, getVectorInfo, purgeMapInfoResults, showMapinfoMarker, hideMapinfoMarker, showMapinfoRevGeocode, hideMapinfoRevGeocode, noQueryableLayers, clearWarning, toggleMapInfoState} = require('../actions/mapInfo');
const {closeAnnotations} = require('../actions/annotations');
const {changeMousePointer} = require('../actions/map');
const {changeMapInfoFormat, updateCenterToMarker} = require('../actions/mapInfo');
const {changeMapInfoFormat, updateCenterToMarker, closeIdentify} = require('../actions/mapInfo');
const {currentLocaleSelector} = require('../selectors/locale');

const {compose, defaultProps} = require('recompose');
Expand Down Expand Up @@ -171,6 +171,7 @@ const IdentifyPlugin = compose(
sendRequest: getFeatureInfo,
localRequest: getVectorInfo,
purgeResults: conditionalToggle,
closeIdentify,
changeMousePointer,
showMarker: showMapinfoMarker,
noQueryableLayers,
Expand Down

0 comments on commit c483be2

Please sign in to comment.