hajkmap
Fix/1204 infoclick title
We've looked into this. The proposed solution (in this PR) will unfortunately not work as it can lead to false positive matches (due to the nature of .includes()).
Imaging this very real example of a group layer consisting of sublayers:
"layers": [
"fastighetsgranser",
"fastighetsytor",
"fastighet",
],
The proposed solution would bring the same caption for all the first and last sublayer because the loop that looks for matching sublayer would evaluate to true two times. ("fastighet".includes("fastighet") === true as well as "fastighetsgranser".includes("fastighet") === true).
So we need more robust logic in this lookup.
I'm testing this out. Documenting here so we can track this back if problems arise further on.
This is just a limited test of two group layers.
Before this PR
Old Infoclick
Group that always worked
Group that doesn't work
New Infoclick
Group that always worked
Group that doesn't work
After this PR
Old Infoclick
Group that always worked
Group that didn't work
New Infoclick
Group that always worked
Group that didn't work and still doesn't work
To sum up the previous post: this solution seems to work but is unfortunately limited to only fix the issue in the "old" infoclick variant. I'm not sure I like to see Hajk diverge this much - it's one thing to add new features that are not backported, but I think that fixing bugs should be done for the entire solution.
Especially since it's easy for an admin to deactivate this fix accidentally, as it's only one checkbox away…
What's your take on this @Hallbergs, @jesade-vbg?
To sum up the previous post: this solution seems to work but is unfortunately limited to only fix the issue in the "old" infoclick variant. I'm not sure I like to see Hajk diverge this much - it's one thing to add new features that are not backported, but I think that fixing bugs should be done for the entire solution.
Especially since it's easy for an admin to deactivate this fix accidentally, as it's only one checkbox away…
What's your take on this @Hallbergs, @jesade-vbg?
I agree that the best solution would be to solve the issue in both the new and old feature-info. @linusfj do you have time to fix it in the new one as well? If not, please make an issue stating that the new one still got the group-bug. (And we can make sure that it is fixed before the next release).
I will try to fix it in the new infoclick as well and add commits to the branch of the pull request.
I've merged your latest changes and tested, but I'm not sure that you verified how it works with the new infoclick. I'll see what I can do to fix this:
