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

oms not working for AdvancedMarkerElement #180

Open
umeWaheed opened this issue Mar 1, 2024 · 17 comments
Open

oms not working for AdvancedMarkerElement #180

umeWaheed opened this issue Mar 1, 2024 · 17 comments

Comments

@umeWaheed
Copy link

umeWaheed commented Mar 1, 2024

Google maps recently deprecated Marker and introduced AdvancedMarkerElement. Using spiderfier for this new marker throws exceptions for the following methods:

  • getVisible
  • getPosition
  • getZIndex
  • setPosition
  • setZIndex
@ericberman
Copy link

Yes, please address! I've found no other good solutions. Shame on Google for deprecating Marker and breaking this with the "upgrade"

@umeWaheed
Copy link
Author

umeWaheed commented Mar 4, 2024 via email

@ericberman
Copy link

I was just upvoting the issue; I'm not familiar enough with coffee to be able to make the fix myself. @jawj - I'm hoping you can provide guidance if not a fix!

@ericberman
Copy link

I may have a fix for this. Testing right now.

@ericberman
Copy link

Pull request submitted

ericberman referenced this issue in ericberman/OverlappingMarkerSpiderfier Mar 19, 2024
Changes:
 * Google didn't provide an equivalent for getVisible in AdvancedMarkerElement; I'm not quite sure how this was intended to be used - whether it was asking "is this marker's location within the bounds of the map" or "is this marker status visible vs. invisible".  So I created a new method IsVisibleMarker that returns true if the marker has a non-null map (map == null is how you remove a marker from the map) AND is within the bounds of that maps latitude/longitude boundaries.  This works for my purposes, I hope it is the correct definition
 * changed getPosition/setPosition to .position property
 * changed getZIndex/setZIndex to .zIndex property
 * commented out the line "(return i if o is obj) for o, i in arr" in p.arrIndexOf.  If I'm understanding Coffee correctly (I'm not!!) this never gets called if arr.indexOf is defined, so this seems to work...

Probably worth noting a few other things I had to do to migrate to AdvancedMarkerElement:
 * No longer use defered script include, so I no longer get a callback when it is loaded, so the old code of waiting for both oms and maps to load no longer works.    E.g., now I load googlemaps with        (g => { var h, a, k, p = "The Google Maps JavaScript API", c = "google", l = "importLibrary", q = "__ib__", m = document, b = window; b = b[c] || (b[c] = {}); var d = b.maps || (b.maps = {}), r = new Set, e = new URLSearchParams, u = () => h || (h = new Promise(async (f, n) => { await (a = m.createElement("script")); e.set("libraries", [...r] + ""); for (k in g) e.set(k.replace(/[A-Z]/g, t => "_" + t[0].toLowerCase()), g[k]); e.set("callback", c + ".maps." + q); a.src = `https://maps.${c}apis.com/maps/api/js?` + e; d[q] = f; a.onerror = () => h = n(Error(p + " could not load.")); a.nonce = m.querySelector("script[nonce]")?.nonce || ""; m.head.append(a) })); d[l] ? console.warn(p + " only loads once. Ignoring:", g) : d[l] = (f, ...n) => r.add(f) && u().then(() => d[l](f, ...n)) })({
           key: "...",
           v: "weekly",
           // Use the 'v' parameter to indicate the version to use (weekly, beta, alpha, etc.).
           // Add other bootstrap parameters as needed, using camel case.
       });

So now I need to create the  overlappingmarkerspiderfier in oogle.maps.event.addListenerOnce(this.gmap, 'idle', function () { ...}
@remcokalf
Copy link

Upvote, we are also waiting on this, before we can move our Maps implementation to Advanced Markers.

@ericberman
Copy link

Want me to send you what I have so you can test it?

@remcokalf
Copy link

Thanks you for the offer. But as things stand now, we prefer to include the library as a whole, not a fork or a customized version. Unless this is not going to be fixed by the maintainers of course. There are no info yet about when Google will actually drop legacy markers, but it will happen at some point, so I hope this gets fixed, rather sooner than later.

@copiri-six
Copy link

Given that the latest update to this repo was in January 2019, I assume that it is no longer maintained.

I have forked the code and updated it to work with AdvancedMarkerElement... if anyone wants the script, you can check out the repo here: https://github.com/copiri-six/AME_Spiderfier.

caveat emptor: I only did the bare minimum in my fork to get the code to work for me. Your mileage may vary... but feel free to contribute over there if you'd like to see further improvements.

@ericberman
Copy link

Nice - thanks!!!

@copiri-six
Copy link

And I should have said... thanks @ericberman for getting the work started, your commit mentioned above was 90% of the work required. Much appreciated!

@remcokalf
Copy link

This is great! thank you!

@jawj
Copy link
Owner

jawj commented May 22, 2024

Sorry for the long silence here. I don't use this library myself any more, but I'll try to integrate these fixes soon.

@remcokalf
Copy link

@jawj that would be awesome! We're using it in FacetWP.com Google map integration, and we'd like to move to Advanced Markers.

@JoelLisenby
Copy link

Thanks all, we were just discussing this one within the last week and it's appreciated that you're upgrading it to advanced markers.

@dCogs
Copy link

dCogs commented Jul 6, 2024

This saved us a ton, thanks!

@jaydiazzz
Copy link

+1

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

No branches or pull requests

8 participants