feat(google-maps): Add Layer components.

[mbehrlich]

Implement components to easily add a traffic, transit or bicycling layer
to an Angular Google Map.

[crisbeto]

The code LGTM, but it seems like it's somewhat identical between the different layers. Would it make sense to either have one component where the user passes in what kind of layer they want (e.g. <map-layer type="traffic">) or have a base class that is shared between them?

[jelbourn]

+1 to @crisbeto's comment

[mbehrlich]

The code LGTM, but it seems like it's somewhat identical between the different layers. Would it make sense to either have one component where the user passes in what kind of layer they want (e.g. <map-layer type="traffic">) or have a base class that is shared between them?

I considered this. The main issue is the "autoRefresh" on the traffic layer causes that component to have a different initialization process than the other two layers, and to implement both processes in a single abstract component would defeat the DRY principle we're going for. I also considered a single component that would implement everything like but I decided against this, because I think that the Angular components should mirror the Google Maps JavaScript API as much as possible. If they had done an initialization like:

let trafficLayer = new google.maps.Layer(google.maps.LayerType.TRAFFIC);

then I would make them the same component, but since google maps made them different classes, I think we should follow their convention and make them separate components.

It would still be possible to try make a base class between the transit and bicycling layers, since they are virtually identical. I didn't do that initially since they're so small and there's only two of them, so the amount of duplicated code is minimal, what do you think?

[jelbourn]

Could it be one layer base class with the traffic layer adding a bit extra in the subclass?

[mbehrlich]

Could it be one layer base class with the traffic layer adding a bit extra in the subclass?

It could, and I've experimented with it a little bit. Ultimately I don't think it's a useful abstraction. Since traffic class is instantiated in a different way, we wouldn't be able to avoid duplication there and ultimately there's very little code that we can avoid duplication for among all of the components. The final code would actually be somewhat longer if we implemented this.

[jelbourn]

Ack- does a shared class just make sense for the other two layers, then?

[mbehrlich]

Done - Added a base class for transit and bicycling layers.

[crisbeto]

LGTM with one nit.

[jelbourn]

@mbehrlich can you rebase? I merged a couple other maps PRs so there's a merge conflict now

[mbehrlich]

@mbehrlich can you rebase? I merged a couple other maps PRs so there's a merge conflict now

Done

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}