#10545: Option to disable identify popup in case of no results #10557

mahmoudadel54 · 2024-09-18T15:50:08Z

Description

In this PR, it is implemented adding an option for identify plugin called 'hidePopupIfNoResults' = true to handle hide identify popup in case of no results for 'openlayers' and 'leaflet' for 2D maps.

Please check if the PR fulfills these requirements

The commit message follows our guidelines: https://github.com/geosolutions-it/MapStore2/blob/master/CONTRIBUTING.md
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (check one with "x", remove the others)

Issue

#10545

What is the current behavior?
#10545

What is the new behavior?
If 'hidePopupIfNoResults' is added to cfg in identify plugin by true ---> the identify popup will be hidden if there is no results.

Breaking change

Does this PR introduce a breaking change? (check one with "x", remove the other)

Yes, and I documented them in migration notes
No

Other useful information

To test it, for desktop add 'hidePopupIfNoResults' with true for cfg in identify plugin.
For mobile, you need to add the identify plugin option 'hidePopupIfNoResults' with true for cfg and edit ---> DEFAULT_VISUALIZATION_MODES_CONFIG.[VisualizationModes._2D].mobile to MapLibraries.LEAFLET in this part below

MapStore2/web/client/utils/MapTypeUtils.js

Lines 27 to 36 in 6c8e852

    
           const DEFAULT_VISUALIZATION_MODES_CONFIG = { 
        
               [VisualizationModes._2D]: { 
        
                   mobile: MapLibraries.OPENLAYERS, 
        
                   desktop: MapLibraries.OPENLAYERS 
        
               }, 
        
               [VisualizationModes._3D]: { 
        
                   mobile: MapLibraries.CESIUM, 
        
                   desktop: MapLibraries.CESIUM 
        
               } 
        
           };

… results Description: - handle adding an option called 'hidePopupIfNoResults' to hide the identify popup - add unit tests - add jsdoc

MV88

there is something unclear to me with the current implementation as i have not yet tested it.

will do another set of review later on, for now i leave some comments as a reminder

MV88 · 2024-09-20T16:27:39Z

web/client/components/data/identify/enhancers/identify.js

@@ -91,6 +93,9 @@ export const identifyLifecycle = compose(
                showAllResponses,
                highlight: pluginCfg?.highlightEnabledFromTheStart || false
            });
+            if (hidePopupIfNoResults) {
+                enableHideEmptyPopupOption(true);


can't you just use the hidePopupIfNoResults prop instead of calling enableHideEmptyPopupOption and updating state?

@MV88 the point is the cfg 'hidePopupIfNoResults ' is for identify plugin and I used it in map plugin

MV88 · 2024-09-20T16:34:23Z

web/client/plugins/map/index.js

+                (state) => state.mapPopups && state.mapPopups.hideEmptyPopupOption || false,
+                isMouseMoveActiveSelector,
+                mapInfoRequestsSelector,
+                responsesSelector,
+                (popups, hideEmptyPopupOption, isMouseMoveActive, mapInfoRequests, mapInfoResponses) => {
+                    // create a flag for hide the identify popup in case no results in hover mode
+                    let identifyPopupHidden = false;
+                    if (isMouseMoveActive && mapInfoRequests?.length && mapInfoResponses?.length && hideEmptyPopupOption) {
+                        const format = getDefaultInfoFormatValue();
+                        const invalidResponses = getValidator(format).getNoValidResponses(mapInfoResponses);
+                        const emptyResponses = mapInfoRequests?.length === invalidResponses?.length;
+                        const missingResponses = (mapInfoRequests || []).length - (mapInfoResponses || []).length;
+                        identifyPopupHidden = missingResponses === 0 && emptyResponses && isMouseMoveActive && hideEmptyPopupOption;
+                    }
+                    return {
+                        popups,
+                        identifyPopupHidden


I have also some doubts here tbh, i will deeply check this

MV88 · 2024-09-20T16:35:18Z

web/client/plugins/map/index.js

-                (popups) => ({
-                    popups
-                })), {
+                (state) => state.mapPopups && state.mapPopups.hideEmptyPopupOption || false,


a dedicated selector must be created when you add a new one

MV88 · 2024-09-20T16:41:33Z

web/client/components/map/leaflet/PopupSupport.jsx

+        const {identifyPopupHidden} = this.props;
+        if (identifyPopupHidden) {
+            (this._popups || []).forEach(({popup}) => {
+                popup.off('remove', this.popupClose);
+                popup && this.props.map.removeLayer(popup);
+            });
+            return [];
+        }
+        return this.preparePopups()


i have some doubts about this implementation too.

MV88 · 2024-09-20T16:43:02Z

web/client/components/map/openlayers/PopupSupport.jsx

@@ -59,6 +61,14 @@ export default class PopupSupport extends React.Component {
        this.props.onPopupClose(id);
    }
    renderPopups = () => {
+        const {identifyPopupHidden, map} = this.props;
+        if (identifyPopupHidden) {


maybe the flag hidePopupIfNoResults could have been used to filter out elements?
i have to debug this tbh

MV88

i tested a different approach and using css you can achieve this goal an avoid all the changes you did

.map-popup-ol:has(.hidePopupIfNoResults) {
    display: none;
}

in DefaultViewer add this

    renderEmptyPages = () => {
        const {emptyResponses} = this.getResponseProperties();
        if (this.props.missingResponses === 0 && emptyResponses) {
            if (this.props.hidePopupIfNoResults) {
                return <span className="hidePopupIfNoResults"/>;
            }
            return (
                <Alert bsStyle={"danger"}>
                    <h4><HTML msgId="noFeatureInfo"/></h4>
                </Alert>
            );
        }
        return null;
    }

Then you just have to make sure that the Identify plugin passes down the hidePopupIfNoResults prop

so please redo from scratch the implementation by providing a new pr and closing this

… results Description: - revert changes in popupSupport files for ol and leaflet + related tests files - handle hide popup for map viewer with css for openlayers and leaflet approach - add unit tests - edit map-popup.less file to handle hide the popup

tdipisa · 2024-10-14T12:35:07Z

@ElenaGallo please test this on master and let us know if we can backport.

ElenaGallo · 2024-10-15T15:00:53Z

@tdipisa @mahmoudadel54 is it correct that when 'hidePopupIfNoResults' is set as true, the marker appears instead of the popup?

Use this map and see the video attached:

identify.mp4

mahmoudadel54 · 2024-10-16T10:26:10Z

@tdipisa @mahmoudadel54 is it correct that when 'hidePopupIfNoResults' is set as true, the marker appears instead of the popup?

Use this map and see the video attached:

identify.mp4

Yes correct @ElenaGallo

tdipisa · 2024-10-17T08:47:35Z

I'm sorry @mahmoudadel54 Where we agreed for this? I don't think it is correct. Why should we show a marked alone? Indeed, when the popup is visible the marked is not needed and it is not displayed, of course

Why should we forcibly display it when 'hidePopupIfNoResults' is set as true?

@ElenaGallo remember to cover also the embedded map test.

* Update Changelog for version 2024.02.00 (geosolutions-it#10602) Co-authored-by: github-actions <[email protected]> * Bump commons-io:commons-io from 2.7 to 2.14.0 (geosolutions-it#10593) * Removed react-confirm-button unused dependency (geosolutions-it#10495) * Update release_steps.md (geosolutions-it#10568) * Remove istambul loader (geosolutions-it#10491) * Attempt to remove instambul loader * removed also package dependency * Apply suggestions from code review clean * Remove jsonpath (geosolutions-it#10494) jsonpath is not needed. It is a dependency of geosolutions-it/patcher actually. Having this dependency here is only confusing for dependency analyisis * Fix geosolutions-it#10595 add missing 'FORMAT' parameter to WMTS GetFeatureInfo requests (geosolutions-it#10596) * add missing 'FORMAT' parameter to WMTS GetFeatureInfo requests this is required by the WMTS spec to be the same format as would be used for a GetTile request, and this allows GFI requests to succeed on https://data.geopf.fr/wmts. * fix mapinfo wmts utils test * Fix geosolutions-it#10505 Allow to specify use of proxy or cors at layer level (geosolutions-it#10526) * fix: remove ui element for force proxy and Allow not secure layers * fix: ajax logic changed, autoDetectCORS is set to true by default * new central CORS util file created and used in ajax * checking CORS before adding in common layer file * null check on getProxyUrl * updated individual layer considring to use proxy if needed * avoid proxy cache to update if response is not okey * enable user to add http url, show warning instead of error, warning text updated * test cases updated * fix: resolve conflicts with url check * fixed the failed test * review cesium layers * include add method in model layer * improve http check for openlayers wms layer * fix tests --------- Co-authored-by: allyoucanmap <[email protected]> * Update openId.md (geosolutions-it#10610) * Bump spring-security version to 5.7.12 (fixes geosolutions-it#10611) (geosolutions-it#10612) somehow something in the build already drags this version, and we end up with two conflicting versions of spring-security in the war which results in at least failure to authenticate with basic auth. * Visibility limits - The resolution option is not retained as Limits type geosolutions-it#10391 (geosolutions-it#10598) * Add lib check release step (geosolutions-it#10614) * geosolutions-it#4675 Remove unused code (geosolutions-it#10442) * geosolutions-it#4675 Remove unused code * remove additional code from review * fix tests folder * remove additional unused files * geosolutions-it#10545: Option to disable identify popup in case of no results (geosolutions-it#10557) * geosolutions-it#10545: Option to disable identify popup in case of no results Description: - handle adding an option called 'hidePopupIfNoResults' to hide the identify popup - add unit tests - add jsdoc * geosolutions-it#10545: Option to disable identify popup in case of no results Description: - revert changes in popupSupport files for ol and leaflet + related tests files - handle hide popup for map viewer with css for openlayers and leaflet approach - add unit tests - edit map-popup.less file to handle hide the popup * geosolutions-it#10545: revert unnecessary changes * Fix geosolutions-it#10615 removed eval from marker utils (geosolutions-it#10616) * geosolutions-it#10545: remove marker in case no results + hover identify mode active and hideEmptyPopupOption with true (geosolutions-it#10619) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Lorenzo Natali <[email protected]> Co-authored-by: Landry Breuil <[email protected]> Co-authored-by: RowHeat <[email protected]> Co-authored-by: allyoucanmap <[email protected]> Co-authored-by: mahmoud adel <[email protected]> Co-authored-by: Matteo V. <[email protected]>

* #10545: Option to disable identify popup in case of no results Description: - handle adding an option called 'hidePopupIfNoResults' to hide the identify popup - add unit tests - add jsdoc * #10545: Option to disable identify popup in case of no results Description: - revert changes in popupSupport files for ol and leaflet + related tests files - handle hide popup for map viewer with css for openlayers and leaflet approach - add unit tests - edit map-popup.less file to handle hide the popup * #10545: revert unnecessary changes

ElenaGallo · 2024-10-23T10:58:10Z

Test passed, @mahmoudadel54 please backport to 2024.02.xx. Thanks

… results (geosolutions-it#10557) * geosolutions-it#10545: Option to disable identify popup in case of no results Description: - handle adding an option called 'hidePopupIfNoResults' to hide the identify popup - add unit tests - add jsdoc * geosolutions-it#10545: Option to disable identify popup in case of no results Description: - revert changes in popupSupport files for ol and leaflet + related tests files - handle hide popup for map viewer with css for openlayers and leaflet approach - add unit tests - edit map-popup.less file to handle hide the popup * geosolutions-it#10545: revert unnecessary changes

mahmoudadel54 · 2024-10-23T14:27:10Z

Test passed, @mahmoudadel54 please backport to 2024.02.xx. Thanks

@ElenaGallo
backport is done ---> #10624

…ase of no results (#10624) * #10545: Option to disable identify popup in case of no results (#10557) * #10545: Option to disable identify popup in case of no results Description: - handle adding an option called 'hidePopupIfNoResults' to hide the identify popup - add unit tests - add jsdoc * #10545: Option to disable identify popup in case of no results Description: - revert changes in popupSupport files for ol and leaflet + related tests files - handle hide popup for map viewer with css for openlayers and leaflet approach - add unit tests - edit map-popup.less file to handle hide the popup * #10545: revert unnecessary changes * #10545: remove marker in case no results + hover identify mode active and hideEmptyPopupOption with true (#10619)

geosolutions-it#10545: Option to disable identify popup in case of no…

a2a2112

… results Description: - handle adding an option called 'hidePopupIfNoResults' to hide the identify popup - add unit tests - add jsdoc

mahmoudadel54 added enhancement BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch C044-VLAANDEREN-2024-SUPPORT labels Sep 18, 2024

mahmoudadel54 added this to the 2024.02.01 milestone Sep 18, 2024

mahmoudadel54 requested a review from MV88 September 18, 2024 15:50

mahmoudadel54 self-assigned this Sep 18, 2024

mahmoudadel54 linked an issue Sep 18, 2024 that may be closed by this pull request

Option to disable identify popup in case of no results #10545

Closed

6 tasks

MV88 requested changes Sep 20, 2024

View reviewed changes

MV88 requested changes Oct 3, 2024

View reviewed changes

MV88 approved these changes Oct 4, 2024

View reviewed changes

geosolutions-it#10545: revert unnecessary changes

8330ab6

tdipisa merged commit c5ed4d0 into geosolutions-it:master Oct 14, 2024
6 checks passed

mahmoudadel54 mentioned this pull request Oct 17, 2024

#10545: fix Option to disable identify popup in case of no results #10619

Merged

12 tasks

mahmoudadel54 mentioned this pull request Oct 23, 2024

[Backport 2024.02.xx] - #10545: Option to disable identify popup in case of no results #10624

Merged

tdipisa removed the BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch label Oct 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#10545: Option to disable identify popup in case of no results #10557

#10545: Option to disable identify popup in case of no results #10557

mahmoudadel54 commented Sep 18, 2024

MV88 left a comment

MV88 Sep 20, 2024

mahmoudadel54 Sep 30, 2024

MV88 Sep 20, 2024

MV88 Sep 20, 2024

MV88 Sep 20, 2024

MV88 Sep 20, 2024

MV88 left a comment

tdipisa commented Oct 14, 2024

ElenaGallo commented Oct 15, 2024

mahmoudadel54 commented Oct 16, 2024

tdipisa commented Oct 17, 2024 •

edited

Loading

ElenaGallo commented Oct 23, 2024

mahmoudadel54 commented Oct 23, 2024

	const DEFAULT_VISUALIZATION_MODES_CONFIG = {
	[VisualizationModes._2D]: {
	mobile: MapLibraries.OPENLAYERS,
	desktop: MapLibraries.OPENLAYERS
	},
	[VisualizationModes._3D]: {
	mobile: MapLibraries.CESIUM,
	desktop: MapLibraries.CESIUM
	}
	};

#10545: Option to disable identify popup in case of no results #10557

#10545: Option to disable identify popup in case of no results #10557

Conversation

mahmoudadel54 commented Sep 18, 2024

Description

Issue

Breaking change

Other useful information

MV88 left a comment

Choose a reason for hiding this comment

MV88 Sep 20, 2024

Choose a reason for hiding this comment

mahmoudadel54 Sep 30, 2024

Choose a reason for hiding this comment

MV88 Sep 20, 2024

Choose a reason for hiding this comment

MV88 Sep 20, 2024

Choose a reason for hiding this comment

MV88 Sep 20, 2024

Choose a reason for hiding this comment

MV88 Sep 20, 2024

Choose a reason for hiding this comment

MV88 left a comment

Choose a reason for hiding this comment

tdipisa commented Oct 14, 2024

ElenaGallo commented Oct 15, 2024

mahmoudadel54 commented Oct 16, 2024

tdipisa commented Oct 17, 2024 • edited Loading

ElenaGallo commented Oct 23, 2024

mahmoudadel54 commented Oct 23, 2024

tdipisa commented Oct 17, 2024 •

edited

Loading