react-google-maps-api icon indicating copy to clipboard operation
react-google-maps-api copied to clipboard

Added `MarkerWithLabel` component (based on `markerwithlabel`).

Open BowlingX opened this issue 4 years ago • 11 comments

This PR adds the MarkerWithLabel component that allows you to create a custom marker from a react-component. Fixes #782, fixes #219

I'm unable to run the linter locally as it seems like prettier is missing in the devDependencies, or it is suppose to be done differently?

I decided to extend the current Marker component instead of extending it's current functionality to give the user the choice to include the markerwithlabel dependency or not. I think in this case inheritance makes sense.

Thank you

BowlingX avatar Jun 04 '20 16:06 BowlingX

@BowlingX Sorry, but this library does not use external dependencies, especially which does not support typescript.. You can fork markerwithlabel, and place it in packages/react-google-maps-api-marker-with-label and reuse tsdx setup from other packages to refactor it to typescript, and after that I will publish it and include as dependency to main project. Please continue in same PR.

JustFly1984 avatar Jun 04 '20 16:06 JustFly1984

Thank you for having a look :). Rewriting the library / Adding types for it is out of my scope currently timewise, but I can provide types for it, as it basically just extends google.maps.Marker and takes some more properties (extending google.maps.MarkerOptions). Let me know if this can work as well.

BowlingX avatar Jun 04 '20 17:06 BowlingX

I also tried to use the current version directly from google (@google/markerwithlabel), but it seems there is a bug on the latest version (see https://github.com/googlemaps/v3-utility-library/issues/651). I ran into the same issue.

BowlingX avatar Jun 04 '20 22:06 BowlingX

@BowlingX well, if you clone markerclusterer to packages/react-google-maps-api-marker-with-label, I will help to refactor it to typescript and publish

JustFly1984 avatar Jun 05 '20 21:06 JustFly1984

@BowlingX I've spent a weekend to give some love to the project. I had rebased your PR to 2.0.0 version branch. I have created new package marker-with-label, but had no time to refactor it to TS yet. Will continue then I will have time again. BTW I've published 2.0.0-alpha and would love if you test it.

JustFly1984 avatar Jun 07 '20 17:06 JustFly1984

PS please fix merge conflicts

JustFly1984 avatar Jun 07 '20 17:06 JustFly1984

I can't test the implementation currently as it seems like there are several issues:

VM762:11 Uncaught SyntaxError: Identifier 'InfoBox' has already been declared
    at new Function (<anonymous>)

...and something is triggering and endless loop:

Warning: Maximum update depth exceeded. This can happen when a component calls setState inside useEffect, but useEffect either doesn't have a dependency array, or one of the dependencies changes on every render.
    in ScriptLoaded (created by bound anonymous)
    in bound anonymous (created by ReactExample)
    in Wrapper (created by ReactExample)
    in ReactExample

BowlingX avatar Jun 08 '20 12:06 BowlingX

I have a general question, why not using the new @google packages for the implementations (e.g. @google/markerclustererplus etc.) instead of porting /copying the dependencies. It seems redundant to me and will make maintainability harder espacially when those are maintained by google directly against there map API.

BowlingX avatar Jun 09 '20 14:06 BowlingX

@BowlingX if it does not support types, it is buggy. It is still buggy as it is. It is not possible to be involved in maintenance of google packages. General quality is sucks. I'm trying to improve general dev experience, and use it as learning experience for myself.

JustFly1984 avatar Jun 09 '20 18:06 JustFly1984

Thanks guys. any idea when this will get merged?

sanath-sfs avatar Dec 07 '20 09:12 sanath-sfs

at this point this PR requires cherrypick to another PR to development branch. Sorry I just have no time to do that. If somebody want to be a champion, I will review and merge, and release new version.

JustFly1984 avatar Dec 07 '20 09:12 JustFly1984