Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Popup - Add layer ID and feature ID as data attributes #4992

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

Gustry
Copy link
Member

@Gustry Gustry commented Nov 19, 2024

According to what I understood, using data-attributes looks is better ?
https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes

I see in many places, that in the code, this hidden input value is used.
And then this value need to be split with . to separate layer ID and feature ID :

<input class="lizmap-popup-layer-feature-id" value="polygons_1e227060_6105_4a4a_92f2_11863ebe65f1.0" type="hidden">

I'm proposing
image

lizMap.events.on({
    lizmappopupdisplayed: function (popup) {
        // It should be #popupcontent
        let popupContainer = document.getElementById(popup.containerId);

        if (!popupContainer) return false;

        Array.from(popupContainer.querySelectorAll('div.lizmapPopupContent .lizmapPopupSingleFeature')).map(element => {
            console.log(element.dataset.featureId);
            console.log(element.dataset.layerId);
        });
    }
});

Linked to #4990

Need to check for raster

@Gustry Gustry added popup javascript Pull requests that update Javascript code labels Nov 19, 2024
@github-actions github-actions bot added this to the 3.10.0 milestone Nov 19, 2024
@rldhont
Copy link
Collaborator

rldhont commented Nov 19, 2024

@Gustry good catch!

@mdouchin
Copy link
Collaborator

I fully agree with this modification. We will have break in Javascript scripts trying to find the previous hidden field, but the needed modification will not be hard to do

@Gustry Gustry force-pushed the data-attributes branch 2 times, most recently from a65f294 to 19e4335 Compare November 21, 2024 18:09
@Gustry
Copy link
Member Author

Gustry commented Nov 21, 2024

For a raster, it gives :

<div data-layer-id="local_raster_layer_c4c2ec5e_7567_476b_bf78_2b7c64f32615" data-feature-id="" class="lizmapPopupSingleFeature">

Do we want it empty, or not available ?

@Gustry Gustry added run end2end If the PR must run end2end tests or not backport release_3_8 backport release_3_9 labels Nov 21, 2024
@Gustry Gustry force-pushed the data-attributes branch 2 times, most recently from a2c3cb3 to 52a6979 Compare November 21, 2024 18:14
@Gustry Gustry force-pushed the data-attributes branch 2 times, most recently from 9c69dbe to 97f911d Compare November 25, 2024 16:37
@Gustry Gustry marked this pull request as ready for review November 25, 2024 16:37
@Gustry
Copy link
Member Author

Gustry commented Nov 25, 2024

PR ready for review @mind84 @rldhont @mdouchin @nboisteault

@Gustry Gustry merged commit f2d3c82 into 3liz:master Nov 26, 2024
13 checks passed
@Gustry Gustry deleted the data-attributes branch November 26, 2024 11:16
@mind84
Copy link
Contributor

mind84 commented Nov 26, 2024

I'll probably have to modify several custom js but it's a more than welcome change 😄

Thanks!

@Gustry
Copy link
Member Author

Gustry commented Nov 26, 2024

No problem.
Indeed, I'm also wondering how we will advertise these deprecated changes.
(and when ...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release_3_8 backport release_3_9 javascript Pull requests that update Javascript code popup run end2end If the PR must run end2end tests or not
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants