#10489: Fix 10438: Problems with GeoStory map configurations merge process#10516
Conversation
| const isLegacyGeostory = (map?.layers || [])?.indexOf(null) !== -1 || (map?.groups || [])?.indexOf(null) !== -1; | ||
| // 'layersGroupsHasEmptyItems' is a flag that indicates if the geostory 'map' object in section includes empty layers/groups | ||
| // the layers/groups array may include empty items in case change layer properties e.g: opacity,title ...etc | ||
| const layersGroupsHasEmptyItems = (map?.layers || []).includes(undefined) || (map?.groups || []).includes(undefined); |
There was a problem hiding this comment.
This implementation seems to solve the problem but it introducing a new set of checks for a specific configuration caused from an inconsistency on the way we update layers/groups in the settings panel.
I would like to propose a change inside the MapEditor instead. The onChange of the Editor component should update the layers or groups key of the map instead to target a specific index of the list of layers/groups. Something like the following code:
<Editor
closeNodeEditor={closeNodeEditor}
editNode={editNode}
map={map}
onChange={(path, value) => {
const config = set({ map: { ...map } }, path, value);
const { layers, groups } = config?.map || {};
if (path.includes('map.layers[')) {
onChangeMap('layers', layers);
} else {
onChangeMap('groups', groups);
}
}}/>By modifying the way we update the layers/groups keys of the map on the setting we are going to align it to the behaviour of the TOC. If we are introducing this change we don't need the layersGroupsHasEmptyItems introduced by this PR
…ry map configurations merge process Description: - fix issue of crash app in geostory if user edit layer properties of existing geostory - add unit tests due to changes
…ry map configurations merge process Description: - implament new approach in onChange for Editor in MapEditor file to fix the app crash issue if user edit layer properties of existing geostory - remove the prev implemented approach - remove unused unit tests
…ry map configurations merge process Description: - remove unused unit tests
b97648e to
84a9760
Compare
|
@ElenaGallo please test this on dev and let us know when we can backport on 2024.02.xx, thanks |
|
Test passed, @mahmoudadel54 please backport to 2024.02.xx. Thanks |
…ations merge process (geosolutions-it#10516)
@ElenaGallo |
Description
This PR fixes the issue that was mentioned in (#10426 (comment)). The issue was in this PR: #10438 in editing this part:
MapStore2/web/client/components/geostory/common/enhancers/map.jsx
Lines 40 to 44 in 7b1e9b4
plus 'createMapObject' method to handle legacy geostory. This editing was missing handling a case of editing layers single propery like layer's title/opacity/visibility ...etc.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x", remove the others)
Issue
#10426 (comment)
What is the current behavior?
(#10426 (comment))
What is the new behavior?
The app doesn't crash.
Breaking change
Does this PR introduce a breaking change? (check one with "x", remove the other)
Other useful information