farmOS / farmOS-map

farmOS-map is an OpenLayers wrapper library designed for agricultural mapping needs. It can be used in any project that has similar requirements.
https://farmOS.github.io/farmOS-map
MIT License
37 stars 22 forks source link

Selected base layer is not being remembered for some users #65

Open yenzand opened 4 years ago

yenzand commented 4 years ago

Hi, Mike, I have a Farmier subscription. I noticed an update this morning - noticeably the dashboard layout. The issue I have is that the base layers on maps are defaulting to Mapbox Satellite. I prefer Google Hybrid because the images are much clearer / accurate and which is what was selected for the base layers. I have tried selecting Google Hybrid but when I go back to a page it displays Mapbox Satellite again. Not sure about the rest of the world but Mapbox Satellite images of New Zealand are very outdated and not great quality. Also some of the borders of areas I have drawn are not accurate on Mapbox Satellite. Is there a default setting I need to change or can this be fixed from you end? Many thanks George

mstenta commented 4 years ago

Hi @yenzand - thanks for opening this issue. There have been some changes to the way base layers work in the upcoming farmOS 7.x-1.3 release. Hopefully they are working as intended. I'll explain what the expected behavior is and perhaps you can tell me if you are experiencing something different.

In the previous version of farmOS, there were only two base layer options: Open Street Map and Google Hybrid. The default was Google Hybrid if a Google API key was provided, otherwise it would be Open Street Map. farmOS also included a setting to set the "sitewide" default, which would apply any time the map was viewed.

In the new version, Open Street Map and Google Hybrid are still there, and there are three additional Google layers (Satellite, Terrain, and Roadmap), and two new Mapbox layers (Satellite and Outdoors).

One difference in the new version is that farmOS no longer includes a setting to set a "sitewide" default base layer. Instead, it defaults to Mapbox Satellite*, and when the base layer is changed, the user's browser remembers which base layer was selected, and uses that as the default everywhere maps are displayed.

* The decision to make Mapbox Satellite the initial default base layer was made for two reasons: 1. the default should be a satellite layer (for obvious reasons), and 2. Google doesn't allow their layers to be used in a standard way with open source mapping libraries like OpenLayers. It requires major workarounds to include the layers, and there are still open issues (eg: https://github.com/farmOS/farmOS-map/issues/59, and https://github.com/mapgears/ol3-google-maps/blob/master/LIMITATIONS.md). So, even though Google's Satellite/Hybrid imagery is better in many cases, its performance is not. The hope is that the "remember selected base layer" functionality would still allow for users to choose Google as their base if they were willing to accept the drawbacks. And moving forward we can consider adding other satellite imagery sources if we want (or create more add-on modules like https://github.com/farmOS/farm_map_no).

I have tried selecting Google Hybrid but when I go back to a page it displays Mapbox Satellite again.

It sounds like the intended behavior I described above ("user's browser remembers which base layer was selected") is not happening in your case. Can you confirm that when you select a different base layer, and you navigate to another page, the selected base layer is not preserved? If so, can you share what browser you are using?

The selected base layer is stored in browser local storage, which you can view in Firefox with Shift+F9 (or going to menu > Web Developer > Storage Inspector), then selecting "Local Storage" on the left.

This behavior is working for me in Firefox, and looks like this in Storage Inspector:

Screenshot from 2020-02-20 21-21-26

If you're able to, I'd be curious to see if yours looks different, or if those are not being saved at all.

Thanks again for bringing this up @yenzand - I'm sure others will have a similar question regarding the new default, so hopefully this is helpful to them as well.

yenzand commented 4 years ago

Hi Mike. Thank you for the update. I am using Firefox 73.0.1 - Win 10. I am getting inconsistent results when navigating between pages - the base layer is remembered for a number of page changes and then the base layer reverts back to Mapbox Satellite. There doesn't seem to be a fixed number of page changes before this happens. When the base layer is remembered the Local Storage is the same as the image in your comment. I will keep an eye on this for the next few days and update you.

I have tried with Chrome 80.0.3987.116 and it appears that the base layer is remembered.

Logging out of FarmOS and then back in resets the base layer to Mapbox Satellite but I take it that's a function of the browser?

mstenta commented 4 years ago

Thank you @yenzand for helping to debug this!

the base layer is remembered for a number of page changes and then the base layer reverts back to Mapbox Satellite. There doesn't seem to be a fixed number of page changes before this happens.

That is very strange!

Logging out of FarmOS and then back in resets the base layer to Mapbox Satellite but I take it that's a function of the browser?

Hmm that's interesting too. That does not happen to me. If I log out and log back in, it uses the previously saved base layer.

I am using Firefox 72.0.2 on Ubuntu Linux.

I will keep an eye on this for the next few days and update you.

Thank you, please do! I'm curious to understand what circumstances cause this.

mstenta commented 4 years ago

We've had another report of this issue in the farmOS chat room. Same basic problem: doesn't work in Chrome.

They also tested the standalone map here: https://farmos.github.io/farmOS-map/ - with the same issue.

I'm going to change the name of this and move it to the farmOS-map repository.

mstenta commented 4 years ago

I wonder if using Babel to transpile would solve this? #61

It's odd because it works for me and @jgaehring in Chrome on Ubuntu and Android. And Chrome has supported localStorage since version 4: https://developer.mozilla.org/en-US/docs/Web/API/Window/localStorage

mstenta commented 4 years ago

Other thoughts:

  1. localStorage must use strings... not booleans. we may be inadvertently saving a boolean instead of "true" or "false"?
  2. farmOS-map is doing localStorage.setItem(), but maybe it needs to be window.localStorage.setItem()
jgaehring commented 4 years ago

localStorage must use strings... not booleans. we may be inadvertently saving a boolean instead of "true" or "false"?

localStorage.setItem() should convert bools to strings automatically. The problem I've encountered before, however, is that strings are not converted back to bools automatically with localStorage.getItem(). For this reason, I often do it like this:

const osmIsVisible = JSON.parse(localStorage.getItem('farmOS.map.layers.OpenStreetMap.visible'));

You can see the difference firsthand if you run that in the console on farmos.github.io/farmOS-map:

image

Bear in mind, too, that in JavaScript !!'false' evaluates to true.

farmOS-map is doing localStorage.setItem(), but maybe it needs to be window.localStorage.setItem()

I seriously doubt that. In even the oldest browsers, any method or property of the window object should be accessible as a global, w/o explicitly calling window.

mstenta commented 4 years ago

That does seem like a likely culprit. Thanks @jgaehring! I'll test this out and push to my fork preview so that others can test it.

mstenta commented 4 years ago

localStorage.setItem() should convert bools to strings automatically.

I wonder if this is in fact the culprit. It does convert them to strings automatically in my tests... but maybe that's NOT happening for the users who reported issues. That could explain the symptoms.

I pushed a simple one-line fix to test: https://github.com/mstenta/farmOS-map/commit/2784b4e7a3182db6ec94795167a821b454f07f0f

Test here: https://mstenta.github.io/farmOS-map/

Explanation:

The code was originally loading the visible property of the layer and saving that directly to localStorage. However, that visible property is a boolean, not a string (reference: https://openlayers.org/en/latest/apidoc/module-ol_layer_Base-BaseLayer.html):

const visible = layer.get('visible');
const itemName = `farmOS.map.layers.${title}.visible`;
localStorage.setItem(itemName, visible);

I changed the first line so that it ensures a string is always saved:

const visible = layer.get('visible') ? 'true' : 'false';
const itemName = `farmOS.map.layers.${title}.visible`;
localStorage.setItem(itemName, visible);
mstenta commented 4 years ago

@yenzand can you test this URL? https://mstenta.github.io/farmOS-map/

Change the base layer, then refresh the page. Does it remember your selected base layer? Please test in the same browser you were having trouble with originally.

I am asking @zedrckr in the farmOS chat to do the same - they also experienced this issue.

jgaehring commented 4 years ago

@mstenta, I think the real issue is this line:

https://github.com/farmOS/farmOS-map/blob/a101a3e6c55ba857205e4b645742cc6bf4f706fa/src/behavior/rememberLayer.js#L11

That should be

const visible = (localStorage.getItem(itemName) === true);

without the quotes around true.

I think if you revert https://github.com/mstenta/farmOS-map/commit/2784b4e7a3182db6ec94795167a821b454f07f0f to the original but make sure to remove those quotes at line 11, it will still work. That seems the more correct solution, imo.

I guess the only remaining question is, how was this working at all for you and me?

jgaehring commented 4 years ago

Or perhaps, to be safe:

const visible = (JSON.parse(localStorage.getItem(itemName)) === true);
jgaehring commented 4 years ago

I guess the only remaining question is, how was this working at all for you and me?

Thinking about this more, my only guess is that some browsers might be parsing the value to a boolean, automatically, upon calling localStorage.getItem(), whereas other browsers are leaving it as a string and requiring you to explicitly run it through JSON.parse(). In fact, looking at the original line 11, my guess is that Chrome was not automatically parsing it as a boolean, whereas Firefox 73 was parsing it as a boolean. The latter case of Firefox 73 would thereby evaluate localStorage.getItem(itemName) === 'true') false, whereas Chrome would evaluate that statement to true. Wrapping the expression in JSON.parse() would guarantee that in both circumstances, given a stored value of 'true', it would evaluate to true no matter the how the browser parses it when taking it out of localStorage.

jgaehring commented 4 years ago

In fact, looking at the original line 11, my guess is that Chrome was not automatically parsing it as a boolean, whereas Firefox 73 was parsing it as a boolean.

Ok, I had to test this out a bit, but here's the result I got from running it in Firefox 73 on my Ubuntu device:

image

That's pretty much equivalent to my test in Ubuntu Chrome 80 (see above https://github.com/farmOS/farmOS-map/issues/65#issuecomment-592161020). So I'm totally baffled now. I still feel like wrapping every localStorage.getItem() in JSON.parse() is the safest, most consistent policy for handling boolean/JSON-ish/non-string values we want to store in localStorage; however, if we were going to be really consistent, we'd make sure to call JSON.stringify() on any values we initially passed in to localStorage.setItem() as well. Anyways, I still don't know what's accounting for this inconsistency in behavior, unless Windows 10 Firefox 73 implements localStorage differently than Ubuntu 18.04 Firefox 73.

:man_shrugging:

I give up.

yenzand commented 4 years ago

@yenzand can you test this URL? https://mstenta.github.io/farmOS-map/

Change the base layer, then refresh the page. Does it remember your selected base layer? Please test in the same browser you were having trouble with originally.

I am asking @zedrckr in the farmOS chat to do the same - they also experienced this issue.

@mstenta I have tried changing the base layer to openstreetmap. The first time I refresh the page it remembers the selected base layer. After a random number of page refreshes (sometimes only two) it reverts back to mapbox satellite. This is also true if I change the base layer to mapbox outdoors. There is no option to select google hybrid as a base layer.

mstenta commented 4 years ago

Thank you @yenzand and @jgaehring for the feedback!

The first time I refresh the page it remembers the selected base layer. After a random number of page refreshes (sometimes only two) it reverts back to mapbox satellite.

Wow that is so weird. Especially if it is not consistent and you're not changing anything else between refreshes. That suggests that the state of localStorage is changing without user interaction.

That should be const visible = (localStorage.getItem(itemName) === true); without the quotes around true.

@jgaehring that is testing to see if localStorage is returning a boolean. But my understanding is that localStorage only stores strings (reference: https://stackoverflow.com/a/3263248).

Or perhaps, to be safe: const visible = (JSON.parse(localStorage.getItem(itemName)) === true);

That makes sense: it would interpret "true" (string) as true (boolean), and then === would match.

But: I'm still trying to understand why that would be necessary. If we store the string "true" in localStorage, I assume that it will come back as the string "true", not the boolean true. Is that correct? Or is that some weird thing about localStorage? Does it convert the string "true" to a boolean in getItem()? That would be weird (and it's JS so I guess I won't be surprised haha).

My original intention with the code was to explicitly store "true" and "false" as strings, and explicitly look for those strings when they were read back into code.

But: OpenLayers upstream stores the layer's visible property as a boolean (see OL docs). So the change I made now explicitly converts the boolean that OpenLayers returns to a string before storing it in localStorage.

That is to say: I don't think we should revert it - even if it isn't the cause of this problem - it's an improvement.

But maybe there is something funky going on in localStorage.getItem() - in some contexts(/browsers/versions/OSes?) - and wrapping it in JSON.parse() will resolve that!

I wish we could replicate it!

jgaehring commented 4 years ago

My original intention with the code was to explicitly store "true" and "false" as strings, and explicitly look for those strings when they were read back into code.

Hm, yea, that in itself seems a little weird to me. I guess there's nothing technically wrong with that, but it just seems to me that, if you're going to treat strings as booleans, you might as well go the whole distance and serialize/deserialize actual JavaScript booleans, via JSON.stringify() and JSON.parse(). If for no other reasons because it makes your intentions clear to anyone one else reading the code exactly what you're doing, and how your doing it, with fewer chances of strings getting accidentally propagated down the call stack to some function that's expecting a boolean. In other words, perform that serialization/deserialization as explicitly as you can, and the closest you can to the actual storage medium.

But maybe there is something funky going on in localStorage.getItem() - in some contexts(/browsers/versions/OSes?) - and wrapping it in JSON.parse() will resolve that!

Yea, it might be redundant or unnecessary, but we at least no one thing:

JSON.parse("true") === JSON.parse(true);

So yea, it couldn't hurt. But I'm still baffled.

Are we sure there couldn't be something clearing out localStorage for some reason?

mstenta commented 4 years ago

Are we sure there couldn't be something clearing out localStorage for some reason?

I don't know. That would be useful to know.

@yenzand is that what's happening to you? When you refresh and it doesn't remember the layer, is there anything saved in your localStorage?

@jgaehring I made this into a new fixLocalStorage branch - curious your thoughts: https://github.com/farmOS/farmOS-map/compare/master...mstenta:fixLocalStorage

  1. Convert layer visibility to a string before saving it to localStorage with JSON.stringify(). https://github.com/farmOS/farmOS-map/commit/0b1098ac5e1601d20ee3994c6f59941412fde537
  2. Test localStorage layer visibility is a boolean after parsing it with JSON.parse(), in case some browsers convert it to boolean automatically in localStorage.getItem(). https://github.com/farmOS/farmOS-map/commit/e738cc3a3944029c58639200fb67f8542fe745d0
  3. Explicitly check to see if localStorage.getItem() is not null before syncing it to OpenLayers. https://github.com/farmOS/farmOS-map/commit/2bfb43024b0a9d118df977fc26195a6f05da5f43

That last one jumped out to me as I was making the other two changes. It's a separate getItem() call that is just for checking to see if a value was set in localStorage, which I made more explicit by checking !== null. If getItem() is converting 'false' to a boolean in some circumstances, then that could mean layer.setVisible() was never running to turn certain layers off, while still turning layers on. I don't know if that was causing an issue, but all of these "hopeful fixes" are banking on the fact that getItem() is returning a boolean in some cases.

Do these make sense?

After I hear back from @yenzand on my question above, I can push these to https://mstenta.github.io/farmOS-map for them to test.

jgaehring commented 4 years ago

That all looks pretty good to me.

If getItem() is converting 'false' to a boolean in some circumstances, then that could mean layer.setVisible() was never running to turn certain layers off, while still turning layers on.

Oh good catch. Yea, best to make sure we rule out every possibility. :crossed_fingers: it works!

yenzand commented 4 years ago

@mstenta I have seen local storage "clear" before. I have now refreshed the page a number of times and the base layer is remembered and local storage retains the two mapbox layers and openstreetmap layer.

mstenta commented 4 years ago

@yenzand - oh interesting - are you saying that you are NOT experiencing the problem anymore?

But in the past, you did experience localStorage clearing? But that's not happening anymore?

yenzand commented 4 years ago

@mstenta That is correct - no problems now.

I didn't check local storage today until you asked me to. It was definitely clearing when I raised the issue on the 21st Feb.

yenzand commented 4 years ago

@mstenta Today I am experiencing the same issues as before - base layer not remembered after a random number of screen refreshes. I have attached a screenshot showing that the local storage is also cleared. farmOS Base Layers

jgaehring commented 4 years ago

Hm, that's strange. Nowhere does this library call localStorage.clear() so there should at least be some keys in there, even if they have null values. Unless localStorage is being cleared manually or automatically by the browser (or by a browser extension).

@yenzand, have you set Firefox to delete cookies and site data upon closing? You'll find this setting under preferences:

image

Or do you have any browser extensions that remove cookies, site data or browser history?

And I assume you haven't, but just to be thorough, have you manually cleared your site data at some point in between page refreshes?

yenzand commented 4 years ago

@jgaehring Yes, Firefox is set to delete cookies and site data upon closing. Am I correct in assuming that as I am refreshing a page and not closing Firefox the cookies and site data will not be deleted.

I am using Adblock Plus but domain - meeta.farmos.net - is whitelisted.

No, I have not manually cleared site data between page refreshes.

jgaehring commented 4 years ago

Am I correct in assuming that as I am refreshing a page and not closing Firefox the cookies and site data will not be deleted.

I would think not, but maybe? Browsers sometimes do some weird things. :expressionless:

I'll keep thinking about this, and maybe @mstenta has more thoughts.

yenzand commented 4 years ago

@jgaehring Thank you, I appreciate your assistance.

mstenta commented 4 years ago

Wow that is weird. It definitely sounds like something specific to your browser configuration. Are you able to try on a different computer with the same browser @yenzand ?

That said, there definitely appear to be two separate issues here. One with your localStorage, and one with Google Layers being remembered, which was reported by @zedrckr and @Skipper-is in chat.

I'm going to continue using this issue for debugging the Google Layers issue. I have a hunch where to look next for that one, and will hopefully be able to do some debugging with @Skipper-is to test it.

yenzand commented 4 years ago

@mstenta Apologies for the delay. I have used a different Windows laptop with Firefox browser and it doesn't appear to have the same issue. It therefore looks like the issue is with my laptop / browser. I will update you of any changes.

mstenta commented 4 years ago

I merged the three commits I made in the fixLocalStorage branch. Even if they don't fix this issue, they are good improvements and make the code more careful and explicit.

tangix commented 4 years ago

Checking my LocalStorage (macOS 10.15.4 and Chrome 81.0.4044.138) I see the following in LocalStorage: 2020-05-26_07-18-50

I have cleared LocalStorage for the site and now at least can get OpenStreetMap to stick as default.

In rememberLayer.js on line 12 you are doing layer.setVisible(visible); Isn't that bad since the Google map layer names contain spaces?

mstenta commented 4 years ago

Thanks for posting these details @tangix ! We need all the help we can get with this one. :-)

Isn't that bad since the Google map layer names contain spaces?

Huh. Maybe you're onto something! I hadn't thought about that as a potential issue. I can't seem to find any official documentation on localStorage variable naming requirements, but maybe some browser/OS combos don't support spaces in the names? It's odd though because this IS working for some, and NOT for others. But nailing down the exact differences has been hard.

Are you using Windows? And Chrome/Firefox? Edge? Oops just re-read your comment and saw you're on Mac+Chrome.

I would also be curious if https://farmos.github.io/farmOS-map works for you. That is a standalone instance of the farmOS-map JavaScript library, which is isolated from farmOS. Some who have experienced say that it works for them there (remembers the selected Google layer after refresh), but doesn't work in farmOS. Which is also confusing to me because there shouldn't be much difference at all...

@yenzand's symptoms described above make this seem unusual indeed:

Today I am experiencing the same issues as before - base layer not remembered after a random number of screen refreshes.

But who knows... this could actually be a whole set of DIFFERENT causes that lead to the same/similar symptom.

tangix commented 4 years ago

I can't seem to find any official documentation on localStorage variable naming requirements, but maybe some browser/OS combos don't support spaces in the names?

I think LocalStorage allows space in names, but the JavaScript reference in the code using layer (passed in as an argument to the function and used when searching LS) will break if it contains spaces, won't it?

Yes, the stand-alone map works for me.

mstenta commented 4 years ago

I think LocalStorage allows space in names, but the JavaScript reference in the code using layer (passed in as an argument to the function and used when searching LS) will break if it contains spaces, won't it?

Oh hmm I'm not sure I'm following. Can you elaborate a bit and maybe link to the specific lines you're referring to?

Yes, the stand-alone map works for me.

Ah but that's interesting, because it's using the same code as farmOS (unless I'm overlooking something). So I'm confused as to why it works in one context but not the other. Others have reported the same thing. 🤔

tangix commented 4 years ago

Oh hmm I'm not sure I'm following. I see why - I was wrong :-)

mstenta commented 4 years ago

The mystery continues! :-)

I really wish I could replicate this myself. It's so strange that it would work in the standalone map but not in farmOS.

mstenta commented 4 years ago

I keep thinking it might be a localStorage issue, but it LOOKS like your localStorage is all correct in farmOS... so maybe it's actually something else entirely, and localStorage is a red herring...

mstenta commented 4 years ago

Oh hmm... a hunch... I wonder if the behaviors are being loaded in a different order in the two contexts. eg: maybe the Google behavior is being run AFTER the rememberLayer behavior? Those pieces are a bit different in farmOS than in farmOS-map standalone.

tangix commented 4 years ago

Yeah, definitely different in the two. Made a screen-recording: https://www.screencast.com/t/PfppBWMGxAOw

In farmOS when changing layers, the google one isn't put in LS.

mstenta commented 4 years ago

Thank you! That is very helpful to see. I think it would be worth investigating my hunch...

maybe the Google behavior is being run AFTER the rememberLayer behavior?

I still wonder why I'm not experiencing it myself, though. But maybe the order in which the JS is being run is not consistent, and I just haven't experienced it purely by chance.

tangix commented 4 years ago

But maybe the order in which the JS is being run is not consistent, and I just haven't experienced it purely by chance.

Back in the days I had problems with a Dojo javascript app that worked from Europe but not from Australia. The culprit was the load order and the time it took for the parser JS to load. Looking at my waterfall I see that the farmOS javascript loads way ahead of the google maps JS. Could this be something?

2020-05-26_14-21-20

Update: I'm on a 4G connection with pretty high latency.

mstenta commented 4 years ago

Back in the days I had problems with a Dojo javascript app that worked from Europe but not from Australia.

Wow that IS an interesting problem! A regional bug!!

Looking at my waterfall I see that the farmOS javascript loads way ahead of the google maps JS.

So... the load time of the script files themselves might not be the issue... farmOS should wait to run the JS until everything is loaded (although maybe there is something I'm overlooking there).

What I am more curious of is: is the rememberLayer behavior's attach() method running before/after the google behavior's attach() method... and is that different in farmOS vs the standalone map.

https://github.com/farmOS/farmOS-map/blob/91cc8b687e462f56988b25ed579c43981dba24f1/src/behavior/rememberLayer.js#L28

https://github.com/farmOS/farmOS-map/blob/91cc8b687e462f56988b25ed579c43981dba24f1/src/behavior/google.js#L14

In the standalone map, the Google behavior is being added here: https://github.com/farmOS/farmOS-map/blob/91cc8b687e462f56988b25ed579c43981dba24f1/static/index.html#L31

But in farmOS, it's being added here: https://github.com/farmOS/farmOS/blob/a40dc486616599f891fc933c31ce766e9a14b5cd/modules/farm/farm_map/farm_map_google/js/farmOS.map.behaviors.google.js#L4

In both cases, the rememberLayer behavior is always added during the initial map creation:

So that is certainly a key difference that could be the culprit here... need to figure out a good way to test that theory. It's tricky because the farmOS-map.js file that is included in farmOS is bundled/minified, so a bit harder to debug.

mstenta commented 4 years ago

Hmm... I'm not sure though... I think in both cases the google behavior will always be added after the rememberLayer behavior... 🤔

mstenta commented 4 years ago

One other difference is that in farmOS, the google behavior is a "behavior wrapped in a behavior": https://github.com/farmOS/farmOS/blob/7.x-1.x/modules/farm/farm_map/farm_map_google/js/farmOS.map.behaviors.google.js#L4

frakman1 commented 3 years ago

When I installed FarmOS via docker, during installation it prompted for a Mapbox token/API along with a Google Maps API. It said that I can enter them later after installation compeltes. Well, I can't find where to enter the Mapbox token details. I can find where the Google Map API can be entered. I think this is why I keep getting blank maps with no base layer selected like this:

image

I have to keep manually selecting the Google layer before I can see anything. Same thing happens with Chrome and Firefox

mstenta commented 3 years ago

I can't find where to enter the Mapbox token details.

The Mapbox module must be enabled. It is only automatically enabled for you if you enter a key during installation. To enter a key after installation you must enable it yourself. See https://github.com/farmOS/farmOS/issues/412

I have to keep manually selecting the Google layer before I can see anything.

As for this issue, we still haven't pinpointed its cause. If you read through the comments above and discover any new clues, please share them here!

perotta commented 3 years ago

@mstenta I'm facing the same problem (Debian KDE, Chrome and Firefox, farm-7.x-1.7-core.tar.gz) . I'm sorry I'm not yet familiar enough with the project to propose a solution but I might have a clue

i.e. OSM base layer is selected, I refresh the page (F5) and it OSM keeps selected. Here is the local storage on that state: image

Then I select Google Maps Hybrid and get this error: image

After that, if I refresh page F5, there is no base layer selected image

mstenta commented 3 years ago

Thanks for the clue @perotta ! This is very timely actually because someone reported the same error in chat a couple days ago:

https://irc.farmos.org/bot/log/farmOS/2021-06-11#T64622

It may have been related to their Google Maps API key not being activated... But I'm not sure if that is directly related or if there are multiple issues at play here.

I asked for clarification this morning: https://irc.farmos.org/bot/log/farmOS/2021-06-13#T64661

Can you confirm that your Google API key is activated?

If this issue is related to the API key it could explain why we've had trouble replicating it. And if that is causing a JS error on the page that could explain why the rememberLayer behavior is not working. This would be good to confirm as it may be the answer to this riddle!

perotta commented 3 years ago

Hi Michael,

I believe my Google Maps is properly activated. My billing section on GCP is active and I have enabled Maps JavaScript API as you can see on the image below.

image

When I select Hybrid base layer on FarmOS I can see the map properly without any watermark (it doesn't remember the selection but I can select hybrid layer each time I land on the page and then I see the satelite/hybrid perspective)

Should I enable other APIs on GCP? (like Maps Embed API) ?

mstenta commented 3 years ago

Thanks @perotta! I don't think enabling other APIs is necessary - it sounds like the API key issue is unrelated. J.Simon Goodall in chat seems to have fixed their API key issue as well, but they are still seeing the same error that you are, and the Google layer is not being remembered.

Still don't know what the exact cause is... but the fact that you're both seeing the same error is interesting...

farmOS-map.js?qujewl:8 Uncaught TypeError: i.elementFromPoint is not a function
    at e.handleMapBrowserEvent (farmOS-map.js?qujewl:8)
    at e.dispatchEvent (farmOS-map.js?qujewl:1)
    at e.<anonymous> (farmOS-map.js?qujewl:8)

No one else in this thread has reported the same error, however, which is curious... 🤔