Skip to content

[Map] Add support for libraries for Google Bridge, inject provider's SDK (L or google) to dispatched events #2044

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

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

Kocal
Copy link
Member

@Kocal Kocal commented Aug 9, 2024

Q A
Bug fix? no
New feature? yes
Issues Fix #...
License MIT

Follows #2040.

This PR gives the developper access to L (if using Leaflet) or google (if using Google Maps) in dispatched events, so the developper can fully and freely customize the map, their markers (before and after creation), and info windows (before and after creation).

I've added some use cases/examples in respective documentation, but tell me if a better place fits!

Also, please no quick merge this time (even if I like that :D), I really wants some reviews for this PR! cc @javiereguiluz 🙏🏻

Code and screenshots

On my personal project, I have a map with a lot of markers.

Before UX Map, I was using Google Maps because I've found the "glyph" feature really sexy, but I was not able to use it anymore... until now!

With Google Maps

Code:

import {Controller} from "@hotwired/stimulus";

export default class extends Controller
{
    connect() {
        this.element.addEventListener('ux:map:marker:before-create', this._onMarkerBeforeCreate);
    }

    disconnect() {
        this.element.removeEventListener('ux:map:marker:before-create', this._onMarkerBeforeCreate);
    }

    _onMarkerBeforeCreate(event) {
        const { definition, google } = event.detail;

        const pinElement = new google.maps.marker.PinElement({
            glyph: new URL(String(definition.extra['icon_mask_uri'])), // I've filled `extra` parameter from `new Marker()` (PHP) with the icon mask URL 
            glyphColor: "white",
        });

        definition.rawOptions = {
            content: pinElement.element,
        }
    }
}

Screenshot:
Capture d’écran 2024-08-09 à 22 58 19

With Leaflet

A dumb example taken from the website:

Code:

import {Controller} from "@hotwired/stimulus";

export default class extends Controller
{
    connect() {
        this.element.addEventListener('ux:map:marker:before-create', this._onMarkerBeforeCreate);
    }

    disconnect() {
        this.element.removeEventListener('ux:map:marker:before-create', this._onMarkerBeforeCreate);
    }

    _onMarkerBeforeCreate(event) {
        const { definition, L } = event.detail;
        
        const redIcon = L.icon({
            iconUrl: 'https://leafletjs.com/examples/custom-icons/leaf-red.png',
            shadowUrl: 'https://leafletjs.com/examples/custom-icons/leaf-shadow.png',
            iconSize:     [38, 95], // size of the icon
            shadowSize:   [50, 64], // size of the shadow
            iconAnchor:   [22, 94], // point of the icon which will correspond to marker's location
            shadowAnchor: [4, 62],  // the same for the shadow
            popupAnchor:  [-3, -76] // point from which the popup should open relative to the iconAnchor
        })
        
        definition.rawOptions = {
            icon: redIcon,
        }
    }
}

Screenshot:

Capture d’écran 2024-08-09 à 23 19 23

@carsonbot carsonbot added Feature New Feature Status: Needs Review Needs to be reviewed labels Aug 9, 2024
@Kocal Kocal force-pushed the feat/ux-map-libraries branch 2 times, most recently from 5ad48e6 to a9b4ca7 Compare August 9, 2024 21:23
@Kocal Kocal requested review from kbond, smnandre and WebMamba August 9, 2024 21:24
@Kocal
Copy link
Member Author

Kocal commented Aug 9, 2024

@simondaigre I didn't forget about you #1937 (comment) ;)

@Kocal Kocal force-pushed the feat/ux-map-libraries branch from a9b4ca7 to bdbc30f Compare August 9, 2024 21:38
@Kocal
Copy link
Member Author

Kocal commented Aug 10, 2024

I've updated the PR to:

  1. not use the deprecated function loader.load(), instead I've used loader.importLibrary() but it requires to re-built the google.maps object structure ourselves (e.g.: you expect PinElement to be accessed from google.maps.marker.PinElement, not from google.maps.PinElement)
  2. to reduce the number of function calls (this.augmentEventPayload()) when events are dispatched N times (N = preconnect + connect + 2 * markers + 2 * infoWindows). Using a protected property looks much more efficient.

@Kocal Kocal changed the title [Map] Add support for libraries Google Bridge, inject provider SDK (L or google) to dispatched events [Map] Add support for libraries for Google Bridge, inject provider's SDK (L or google) to dispatched events Aug 10, 2024
@smnandre
Copy link
Member

Could you add some shared alias for google and L ? Any better idea than provider or service ?

| `retries` | The number of script load retries | 3 |
| `url` | Custom url to load the Google Maps API script | `https://maps.googleapis.com/maps/api/js` |
| `version` | The release channels or version numbers | `weekly` |
| `libraries` | The additional libraries to load, see [list of supported libraries](https://googlemaps.github.io/js-api-loader/types/Library.html) | `['maps', 'marker']`, those two libraries are always loaded |
Copy link
Member

Choose a reason for hiding this comment

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

Should that be in controller.json or something like that ?

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you mean?

export default class extends Controller
{
connect() {
this.element.addEventListener('ux:map:marker:before-create', this._onMarkerBeforeCreate);
Copy link
Member

@smnandre smnandre Aug 11, 2024

Choose a reason for hiding this comment

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

This is new: ux:map:marker:before-create ?

Or did i left way too long to remember ? (4 days hahaha)

Copy link
Member Author

Choose a reason for hiding this comment

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

Always has been here ;)

Comment on lines +121 to +120
const beachFlagImg = document.createElement("img");
// Note: instead of using an hardcoded URL, you can use the `extra` parameter from `new Marker()` (PHP) and access it here with `definition.extra`.
beachFlagImg.src = "https://developers.google.com/maps/documentation/javascript/examples/full/images/beachflag.png";
Copy link
Member

Choose a reason for hiding this comment

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

I think we should illustrate this example with a controller "value"

Comment on lines +72 to +89
_onMarkerBeforeCreate(event) {
// You can access the marker definition and the Leaflet object
// Note: `definition.rawOptions` is the raw options object that will be passed to the `L.marker` constructor.
const { definition, L } = event.detail;

// Use a custom icon for the marker
const redIcon = L.icon({
// Note: instead of using an hardcoded URL, you can use the `extra` parameter from `new Marker()` (PHP) and access it here with `definition.extra`.
iconUrl: 'https://leafletjs.com/examples/custom-icons/leaf-red.png',
shadowUrl: 'https://leafletjs.com/examples/custom-icons/leaf-shadow.png',
iconSize: [38, 95], // size of the icon
shadowSize: [50, 64], // size of the shadow
iconAnchor: [22, 94], // point of the icon which will correspond to marker's location
shadowAnchor: [4, 62], // the same for the shadow
popupAnchor: [-3, -76] // point from which the popup should open relative to the iconAnchor
})

definition.rawOptions = {
icon: redIcon,
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This will update all maps on the page (if multiple ones) right ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, only the ones using my-map as data-controller


class map_controller extends AbstractMapController {
connect() {
Marker.prototype.options.icon = divIcon({
L.Marker.prototype.options.icon = L.divIcon({
html: '<svg xmlns="http://www.w3.org/2000/svg" xml:space="preserve" fill-rule="evenodd" stroke-linecap="round" clip-rule="evenodd" viewBox="0 0 500 820"><defs><linearGradient id="a" x1="0" x2="1" y1="0" y2="0" gradientTransform="matrix(0 -37.57 37.57 0 416.45 541)" gradientUnits="userSpaceOnUse"><stop offset="0" stop-color="#126FC6"/><stop offset="1" stop-color="#4C9CD1"/></linearGradient><linearGradient id="b" x1="0" x2="1" y1="0" y2="0" gradientTransform="matrix(0 -19.05 19.05 0 414.48 522.49)" gradientUnits="userSpaceOnUse"><stop offset="0" stop-color="#2E6C97"/><stop offset="1" stop-color="#3883B7"/></linearGradient></defs><circle cx="252.31" cy="266.24" r="83.99" fill="#fff"/><path fill="url(#a)" stroke="url(#b)" stroke-width="1.1" d="M416.54 503.61c-6.57 0-12.04 5.7-12.04 11.87 0 2.78 1.56 6.3 2.7 8.74l9.3 17.88 9.26-17.88c1.13-2.43 2.74-5.79 2.74-8.74 0-6.18-5.38-11.87-11.96-11.87Zm0 7.16a4.69 4.69 0 1 1-.02 9.4 4.69 4.69 0 0 1 .02-9.4Z" transform="translate(-7889.1 -9807.44) scale(19.54)"/></svg>',
Copy link
Member

Choose a reason for hiding this comment

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

Oh please remind me to give you a more optimized icon @Kocal ... sorry 😞

Copy link
Member Author

Choose a reason for hiding this comment

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

I recently updated it in #2036, I think we are really fine now

@smnandre
Copy link
Member

I know this is some "last minute before release" PR, but let's try to not mix features (extra info, librairies configuration) and documentation (custom marker) for the next ones ;)

That beeing said: thanks for the job again @Kocal !! Amazing

@Kocal Kocal force-pushed the feat/ux-map-libraries branch 3 times, most recently from 55ab782 to 7faf302 Compare August 12, 2024 05:30
@Kocal
Copy link
Member Author

Kocal commented Aug 12, 2024

but let's try to not mix features (extra info, librairies configuration) and documentation (custom marker) for the next ones ;)

I'm curious, how would you have done it? Dedicated PRs for:

  • Google Maps librairies configuration
  • loader.load() rework
  • abstracting dispatchEvent
  • L and google injection

👀

@Kocal Kocal force-pushed the feat/ux-map-libraries branch 2 times, most recently from 83b8e03 to d54c835 Compare August 12, 2024 19:29
…s SDK (`L` or `google`) to dispatched events
@kbond kbond force-pushed the feat/ux-map-libraries branch from d54c835 to 2dbb169 Compare August 12, 2024 19:36
@kbond
Copy link
Member

kbond commented Aug 12, 2024

Thanks Hugo.

@kbond kbond merged commit 9e6920d into symfony:2.x Aug 12, 2024
4 of 5 checks passed
@Kocal Kocal added the Map label Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New Feature Map Status: Needs Review Needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants