Skip to content

Commit

Permalink
Fix #9120 Layer settings performance improvement (#9121)
Browse files Browse the repository at this point in the history
  • Loading branch information
@offtherailz
offtherailz authored Apr 21, 2023
1 parent f4efd00 commit bd7f030
Show file tree
Hide file tree
Showing 13 changed files with 91 additions and 238 deletions.
35 changes: 0 additions & 35 deletions web/client/components/TOC/enhancers/__tests__/tocitemssettings-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,6 @@ describe("test updateSettingsLifecycle", () => {
setTimeout(done);
});

it('test mounted component', () => {
const testHandlers = {
onUpdateOriginalSettings: () => {},
onUpdateInitialSettings: () => {}
};

const spyOnUpdateOriginalSettings = expect.spyOn(testHandlers, 'onUpdateOriginalSettings');
const spyOnUpdateInitialSettings = expect.spyOn(testHandlers, 'onUpdateInitialSettings');

const Component = settingsLifecycle(() => <div></div>);
ReactDOM.render(<Component
element={{}}
onUpdateOriginalSettings={testHandlers.onUpdateOriginalSettings}
onUpdateInitialSettings={testHandlers.onUpdateInitialSettings}
/>, document.getElementById("container"));

expect(spyOnUpdateOriginalSettings).toHaveBeenCalled();
expect(spyOnUpdateInitialSettings).toHaveBeenCalled();
});

it('test mounted retrieve data layer', () => {
const testHandlers = {
onRetrieveLayerData: () => {}
Expand Down Expand Up @@ -115,35 +95,26 @@ describe("test updateSettingsLifecycle", () => {

it('test component update', () => {
const testHandlers = {
onUpdateOriginalSettings: () => {},
onUpdateInitialSettings: () => {},
onSetTab: () => {}
};
const spyOnUpdateOriginalSettings = expect.spyOn(testHandlers, 'onUpdateOriginalSettings');
const spyOnUpdateInitialSettings = expect.spyOn(testHandlers, 'onUpdateInitialSettings');
const spyOnSetTab = expect.spyOn(testHandlers, 'onSetTab');

const Component = settingsLifecycle(() => <div></div>);

ReactDOM.render(<Component
initialActiveTab="display"
settings={{expanded: false}}
onUpdateOriginalSettings={testHandlers.onUpdateOriginalSettings}
onUpdateInitialSettings={testHandlers.onUpdateInitialSettings}
onSetTab={testHandlers.onSetTab}
/>, document.getElementById("container"));

ReactDOM.render(<Component
initialActiveTab="display"
element={{type: 'wms', description: 'description'}}
settings={{expanded: true}}
onUpdateOriginalSettings={testHandlers.onUpdateOriginalSettings}
onUpdateInitialSettings={testHandlers.onUpdateInitialSettings}
onSetTab={testHandlers.onSetTab}
/>, document.getElementById("container"));

expect(spyOnUpdateOriginalSettings).toHaveBeenCalled();
expect(spyOnUpdateInitialSettings).toHaveBeenCalled();
expect(spyOnSetTab).toHaveBeenCalled();
expect(spyOnSetTab).toHaveBeenCalledWith("display");
});
Expand All @@ -157,7 +128,6 @@ describe("test updateSettingsLifecycle", () => {
const Component = settingsLifecycle(({onClose}) => <div id="test-close" onClick={() => onClose()}></div>);
ReactDOM.render(<Component
onUpdateNode={testHandlers.onUpdateNode}
originalSettings={{}}
settings={{node: '0', nodeType: 'layer', options: {}}}
onHideSettings={testHandlers.onHideSettings}/>, document.getElementById("container"));

Expand All @@ -176,7 +146,6 @@ describe("test updateSettingsLifecycle", () => {

const Component = settingsLifecycle(({onClose}) => <div id="test-close" onClick={() => onClose()}></div>);
ReactDOM.render(<Component
originalSettings={{}}
settings={{node: '0', nodeType: 'layer', options: { opacity: 1.0 }}}
onHideSettings={testHandlers.onHideSettings} />, document.getElementById("container"));

Expand All @@ -194,7 +163,6 @@ describe("test updateSettingsLifecycle", () => {

const Component = settingsLifecycle(({onClose}) => <div id="test-close" onClick={() => onClose()}></div>);
ReactDOM.render(<Component
originalSettings={{}}
settings={{node: '0', nodeType: 'layer', options: { style: 'new-style' }}}
onHideSettings={testHandlers.onHideSettings} />, document.getElementById("container"));

Expand All @@ -216,7 +184,6 @@ describe("test updateSettingsLifecycle", () => {

const Component = settingsLifecycle(({onClose}) => <div id="test-close" onClick={() => onClose(tabCloseActions)}></div>);
ReactDOM.render(<Component
originalSettings={{}}
settings={{node: '0', nodeType: 'layer', options: { style: 'new-style' }}}
/>, document.getElementById("container"));

Expand All @@ -238,7 +205,6 @@ describe("test updateSettingsLifecycle", () => {

const Component = settingsLifecycle(({onClose}) => <div id="test-close" onClick={() => onClose(tabCloseActions)}></div>);
ReactDOM.render(<Component
originalSettings={{}}
settings={{node: '0', nodeType: 'layer', options: { style: 'new-style' }}}
/>, document.getElementById("container"));

Expand All @@ -260,7 +226,6 @@ describe("test updateSettingsLifecycle", () => {

const Component = settingsLifecycle(({onClose}) => <div id="test-close" onClick={() => onClose(tabCloseActions)}></div>);
ReactDOM.render(<Component
originalSettings={{}}
settings={{node: '0', nodeType: 'layer', options: { style: 'new-style' }}}
/>, document.getElementById("container"));

Expand Down
23 changes: 3 additions & 20 deletions web/client/components/TOC/enhancers/tocItemsSettings.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,33 +34,20 @@ export const settingsState = compose(
*/
export const settingsLifecycle = compose(
withHandlers({
onClose: ({ onUpdateInitialSettings = () => {}, onHideSettings = () => {}, onUpdateOriginalSettings = () => {} }) => (tabsCloseActions = []) => {
onClose: ({ onHideSettings = () => {}}) => (tabsCloseActions = []) => {
if (isArray(tabsCloseActions)) {
tabsCloseActions.forEach(tabOnClose => {
if (isFunction(tabOnClose)) {
tabOnClose();
}
});
}
onUpdateOriginalSettings({});
onHideSettings();
// clean up internal settings state
onUpdateInitialSettings({});

}
}),
lifecycle({
UNSAFE_componentWillMount() {
const {
element = {},
onUpdateOriginalSettings = () => { },
onUpdateInitialSettings = () => { }
} = this.props;

// store changed keys
onUpdateOriginalSettings({ });
// store initial settings (internal state)
onUpdateInitialSettings({ ...element });
},
componentWillReceiveProps(newProps) {
// an empty description does not trigger the single layer getCapabilites,
// it does only for missing description
Expand All @@ -77,15 +64,11 @@ export const settingsLifecycle = compose(
const {
initialActiveTab = 'general',
settings = {},
onUpdateOriginalSettings = () => { },
onUpdateInitialSettings = () => { },

onSetTab = () => { }
} = this.props;

if (!settings.expanded && newProps.settings && newProps.settings.expanded) {
// update initial and original settings
onUpdateOriginalSettings({ });
onUpdateInitialSettings({ ...newProps.element });
onSetTab(initialActiveTab);
}
}
Expand Down
5 changes: 1 addition & 4 deletions web/client/containers/Embedded.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/
import React from 'react';
import PropTypes from 'prop-types';
import assign from 'object-assign';
import ModulePluginsContainer from "../product/pages/containers/ModulePluginsContainer";
import { connect } from 'react-redux';

Expand All @@ -18,9 +17,7 @@ import ConfigUtils from '../utils/ConfigUtils';

const PluginsContainer = connect((state) => ({
mode: urlQuery.mode || (state.browser && state.browser.mobile ? 'mobile' : 'desktop'),
pluginsState: assign({}, state && state.controls, state && state.layers && state.layers.settings && {
layerSettings: state.layers.settings
}),
pluginsState: state && state.controls,
monitoredState: getMonitoredState(state, ConfigUtils.getConfigProp('monitorState'))
}))(ModulePluginsContainer);

Expand Down
8 changes: 2 additions & 6 deletions web/client/containers/MapViewer.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,11 @@ const PluginsContainer = connect(
state => state.mode,
state => state?.browser?.mobile,
state => state.controls,
state => state?.layers?.settings,
state => getMonitoredState(state, ConfigUtils.getConfigProp('monitorState')),
(statePluginsConfig, stateMode, mobile, controls, layerSettings, monitoredState) => ({
(statePluginsConfig, stateMode, mobile, controls, monitoredState) => ({
statePluginsConfig,
mode: urlQuery.mode || stateMode || (mobile ? 'mobile' : 'desktop'),
pluginsState: {
...controls,
...(layerSettings && { layerSettings })
},
pluginsState: controls,
monitoredState
})
)
Expand Down
68 changes: 19 additions & 49 deletions web/client/epics/__tests__/layers-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
layerLoad
} from '../../actions/layers';

import { SET_CONTROL_PROPERTY } from '../../actions/controls';
import { SHOW_NOTIFICATION } from '../../actions/notifications';
import { testEpic } from './epicTestUtils';
import { refresh, updateDimension, updateSettingsParamsEpic } from '../layers';
Expand Down Expand Up @@ -144,27 +143,19 @@ describe('layers Epics', () => {
id: 'layerid',
name: 'layerName',
style: ''
},
originalSettings: {

}
}
}
};

testEpic(
updateSettingsParamsEpic,
2,
1,
updateSettingsParams({style: 'generic'}),
actions => {
expect(actions.length).toBe(2);
expect(actions.length).toBe(1);
actions.map((action) => {
switch (action.type) {
case SET_CONTROL_PROPERTY:
expect(action.control).toBe('layersettings');
expect(action.property).toBe('originalSettings');
expect(action.value).toEqual({ style: '' });
break;
case UPDATE_SETTINGS:
expect(action.options).toEqual({style: 'generic'});
break;
Expand All @@ -185,9 +176,6 @@ describe('layers Epics', () => {
id: 'layerId',
name: 'layerName',
style: ''
},
originalSettings: {

}
}
},
Expand All @@ -203,17 +191,12 @@ describe('layers Epics', () => {

testEpic(
updateSettingsParamsEpic,
3,
2,
updateSettingsParams({style: 'generic'}, true),
actions => {
expect(actions.length).toBe(3);
expect(actions.length).toBe(2);
actions.map((action) => {
switch (action.type) {
case SET_CONTROL_PROPERTY:
expect(action.control).toBe('layersettings');
expect(action.property).toBe('originalSettings');
expect(action.value).toEqual({ style: '' });
break;
case UPDATE_SETTINGS:
expect(action.options).toEqual({style: 'generic'});
break;
Expand All @@ -238,9 +221,6 @@ describe('layers Epics', () => {
id: 'layerId',
name: 'layerName',
style: ''
},
originalSettings: {

}
}
},
Expand All @@ -261,20 +241,16 @@ describe('layers Epics', () => {

testEpic(
updateSettingsParamsEpic,
3,
2,
[updateSettingsParams({name: 'layerName_changed'}, true), layerLoad('layerId')],
actions => {
expect(actions.length).toBe(3);
expect(actions.length).toBe(2);
expect(actions[0].type).toBe(UPDATE_SETTINGS);
expect(actions[0].options).toEqual({name: 'layerName_changed'});
expect(actions[1].type).toBe(SET_CONTROL_PROPERTY);
expect(actions[1].control).toBe('layersettings');
expect(actions[1].property).toBe('originalSettings');
expect(actions[1].value).toEqual({name: 'layerName'});
expect(actions[2].type).toBe(UPDATE_NODE);
expect(actions[2].node).toEqual('layerId');
expect(actions[2].nodeType).toEqual('layers');
expect(actions[2].options).toEqual({opacity: 1, name: 'layerName_changed'});
expect(actions[1].type).toBe(UPDATE_NODE);
expect(actions[1].node).toEqual('layerId');
expect(actions[1].nodeType).toEqual('layers');
expect(actions[1].options).toEqual({opacity: 1, name: 'layerName_changed'});
}, state, done);
});

Expand All @@ -286,9 +262,6 @@ describe('layers Epics', () => {
id: 'layerId',
name: 'layerName',
style: ''
},
originalSettings: {

}
}
},
Expand All @@ -309,22 +282,19 @@ describe('layers Epics', () => {

testEpic(
updateSettingsParamsEpic,
4,
3,
[updateSettingsParams({name: 'layerName_changed'}, true), layerLoad('layerId', true)],
actions => {
expect(actions.length).toBe(4);
expect(actions.length).toBe(3);
expect(actions[0].type).toBe(UPDATE_SETTINGS);
expect(actions[0].options).toEqual({name: 'layerName_changed'});
expect(actions[1].type).toBe(SET_CONTROL_PROPERTY);
expect(actions[1].control).toBe('layersettings');
expect(actions[1].property).toBe('originalSettings');
expect(actions[1].value).toEqual({name: 'layerName'});
expect(actions[2].type).toBe(UPDATE_NODE);
expect(actions[2].node).toEqual('layerId');
expect(actions[2].nodeType).toEqual('layers');
expect(actions[2].options).toEqual({opacity: 1, name: 'layerName_changed'});
expect(actions[3].type).toBe(SHOW_NOTIFICATION);
expect(actions[3].level).toBe('error');

expect(actions[1].type).toBe(UPDATE_NODE);
expect(actions[1].node).toEqual('layerId');
expect(actions[1].nodeType).toEqual('layers');
expect(actions[1].options).toEqual({opacity: 1, name: 'layerName_changed'});
expect(actions[2].type).toBe(SHOW_NOTIFICATION);
expect(actions[2].level).toBe('error');
}, state, done);
});
});
15 changes: 4 additions & 11 deletions web/client/epics/__tests__/styleeditor-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,13 @@
*/

import expect from 'expect';
import {find} from 'lodash';

import {
UPDATE_NODE,
UPDATE_SETTINGS_PARAMS
} from '../../actions/layers';

import {
SET_CONTROL_PROPERTY
} from '../../actions/controls';

import {
SHOW_NOTIFICATION
} from '../../actions/notifications';
Expand Down Expand Up @@ -695,17 +692,13 @@ describe('Test styleeditor epics', () => {
code: '* { stroke: #ff0000; }'
}
};
const NUMBER_OF_ACTIONS = 6;
const NUMBER_OF_ACTIONS = 4;
const results = (actions) => {
expect(actions.length).toBe(NUMBER_OF_ACTIONS);
try {
expect(actions[1].type).toBe(UPDATE_SETTINGS_PARAMS);
expect(actions[2].type).toBe(LOADED_STYLE);
expect(actions[3].type).toBe(SET_CONTROL_PROPERTY);
expect(actions[4].type).toBe(SET_CONTROL_PROPERTY);
expect(actions[5].type).toBe(SHOW_NOTIFICATION);

expect(actions[5].level).toBe('success');
expect(find(actions, {type: SHOW_NOTIFICATION}).level).toBe('success');
expect(find(actions, {type: LOADED_STYLE})).toBeTruthy();
} catch (e) {
done(e);
}
Expand Down
Loading

0 comments on commit bd7f030

Please sign in to comment.