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

Allow stateful components with PermalinkProvider #632

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

geographika
Copy link
Member

Possible fix for #631.

Inherit from CookieProvider so other components in the application can save state, and only set map state when map object/value properties are passed.

This has the drawback that the LocalStorageProvider cannot be used, so perhaps redesigning the PermalinkProvider as a mixin to be added to any Provider class would be a better approach?

CC @leonardospina

@chrismayer
Copy link
Contributor

Looks good at first sight. Couldn't test it yet. Feels like a (good) workaround. But we should think of a more general approach, as you already stated.

@geographika
Copy link
Member Author

How about adding the current PermalinkProvider functions to a PermalinkMixin and then adding the mixin to 2 new GeoExt classes deriving from the 2 ExtJS state providers?

E.g. GeoExt.state.CookieProvider and GeoExt.state.LocalStorageProvider

@chrismayer
Copy link
Contributor

chrismayer commented Feb 27, 2020

Creating a mixin and two dedicated class definitions having the mixin sounds like a good idea. If that will work this would be my preferred way to solve this.

Should a third class GeoExt.state.PermalinkProvider also be available or do you think this is way too much? Think on uses cases where you don't want to use cookies or LocalStorage.

@geographika
Copy link
Member Author

Should a third class GeoExt.state.PermalinkProvider also be available or do you think this is way too much? Think on uses cases where you don't want to use cookies or LocalStorage.

I think the mixin would be enough? The GeoExt.state.PermalinkProvider would be empty apart from the mixin, so probably easier to add the mixin to a custom class as and when required.

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.

None yet

2 participants