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

Add 'wgu-geolocator-layer' to permalink ignore list #240

Merged
merged 1 commit into from
Sep 6, 2021

Conversation

chrismayer
Copy link
Collaborator

This adds the layer to show the geolocation on the map to the ignore layer list of the permalink module, since it makes no sense to have it in the permalink and the modules are (currently) not restored.

/cc @justb4 @fschmenger

This adds the layer to show the geolocation on the map to the ignore
layer list of the permalink module.
@fschmenger
Copy link
Collaborator

@chrismayer For now we should also add 'wgu-drag-drop-layer' to the ignore list.

I think a better long term fix for permalink would be to ignore any layer added dynamically on a module level (not via config and factory) - since those most likely wont work.
An implementation would be fairly straightforward:

  • The layer factory will add a custom attribute 'usePermalink' to the OL layer list and default this flag to true (maybe with an option turn it off via config).
  • Permalink controller evaluates this flag (instead of using static ignore list).

Then application code can decide whether it makes sense to flag its own layers as usable for permalink.

@fschmenger
Copy link
Collaborator

Be aware, that 'wgu-drap-drop-layer' contains a random suffix, so it's not a one-liner fix.
I see no problem incorporating the PR. Should we convert the stuff stated above to an issue?

@chrismayer
Copy link
Collaborator Author

chrismayer commented Sep 6, 2021

Yes, I vote for merging this and having an issue for the more general approach. Would you please create such an issue @fschmenger ?

@fschmenger
Copy link
Collaborator

Agreed, merge this for now and I will open an issue.

@chrismayer
Copy link
Collaborator Author

Thanks for the issue tracking @fschmenger!

@chrismayer chrismayer merged commit 67db375 into wegue-oss:master Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants