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

feat: add advanced markers #192

Merged
merged 26 commits into from Apr 16, 2024
Merged

Conversation

colenso
Copy link
Contributor

@colenso colenso commented Apr 15, 2024

Since the standard marker is deprecated.

@colenso
Copy link
Contributor Author

colenso commented Apr 15, 2024

@sandydoo I've made the changes you had requested in the old PR. Please have a look. 🙏
One thing to note is that advanced markers will only work if the user adds the marker library. In this PR, I've not added it by default like I did in the previous PR because I felt that it should only be added if the user actually uses the advanced-marker feature.

@colenso
Copy link
Contributor Author

colenso commented Apr 15, 2024

@sandydoo For some reason the tests that I added aren't passing. I'll try to get them working and you can then review this PR.

@colenso
Copy link
Contributor Author

colenso commented Apr 16, 2024

@sandydoo I've fixed all the tests. Please review. 🙏

addon/components/g-map.js Outdated Show resolved Hide resolved
tests/integration/components/g-map/info-window-test.js Outdated Show resolved Hide resolved
Copy link
Owner

@sandydoo sandydoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! A few more fixes and we'll be good to go

addon/components/g-map.js Outdated Show resolved Hide resolved
tests/integration/components/g-map/info-window-test.js Outdated Show resolved Hide resolved
addon/components/g-map/advanced-marker.js Outdated Show resolved Hide resolved
addon/components/g-map/advanced-marker.js Outdated Show resolved Hide resolved
docs/app/templates/docs/advanced-markers.hbs Show resolved Hide resolved
@colenso
Copy link
Contributor Author

colenso commented Apr 16, 2024

OK. @sandydoo I've done all the requested changes.
One final review. 🙌

addon/components/g-map.js Outdated Show resolved Hide resolved
addon/components/g-map/advanced-marker.js Outdated Show resolved Hide resolved
addon/components/g-map.js Outdated Show resolved Hide resolved
colenso and others added 2 commits April 16, 2024 17:21
Co-authored-by: sander <sandermel94@gmail.com>
@colenso
Copy link
Contributor Author

colenso commented Apr 16, 2024

OK. All tests passing with all your recommendations ✅ @sandydoo

@sandydoo
Copy link
Owner

@colenso, just needs a pnpm lint:fix now

@colenso
Copy link
Contributor Author

colenso commented Apr 16, 2024

@colenso, just needs a pnpm lint:fix now

Done ✅

@sandydoo sandydoo added the enhancement New feature or request label Apr 16, 2024
@sandydoo sandydoo merged commit 24dade1 into sandydoo:main Apr 16, 2024
12 checks passed
@sandydoo
Copy link
Owner

@colenso, thanks a bunch!

@colenso
Copy link
Contributor Author

colenso commented Apr 17, 2024

Thanks @sandydoo Great collaborating with you on this. Could you please release a new version that includes this feature?

@colenso
Copy link
Contributor Author

colenso commented Apr 18, 2024

@sandydoo ☝️ 🙏 🥲

@sandydoo
Copy link
Owner

Released v7.2.1. I've removed the default mapId from the map. I realized when deploying the docs that the mapId:

  1. Is not backwards-compatible with the old way of styling the map.
  2. Must be created in the Google Maps Platform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants