feat(google-maps): Add MapPolyline component #17512

mbehrlich · 2019-10-26T01:28:08Z

Add a MapPolyline component to @angular/google-maps that allows drawing
polylines on a Google Map.

src/google-maps/map-polyline/map-polyline.spec.ts

src/google-maps/map-polyline/map-polyline.ts

crisbeto · 2019-10-26T12:12:55Z

src/google-maps/map-polyline/map-polyline.ts

+   * @see developers.google.com/maps/documentation/javascript/reference/polygon#Polyline.getEditable
+   */
+  getEditable(): boolean {
+    return this._polyline!.getEditable();


From what I can tell, we aren't guaranteed to have a _polyline at all times so the the ! here is misleading. It might be better to null check it. Same goes for all the other getters.

I did it like this because _polyline is initialized in ngOnInit, but it should always be initialized. Let me know if the latest changes I'm uploading are acceptable.

jelbourn

Include docs for polyline in this change?

src/google-maps/map-polyline/map-polyline.ts

mbehrlich · 2019-11-04T19:57:35Z

Include docs for polyline in this change?

If this component isn't going out in the 9.0 release, will that be confusing for users if Polyline is in the docs but not in the release? My intention was to make a separate document for all of the geometry components and have the main docs link to it when they were ready. What do you think?

src/dev-app/google-map/google-map-demo.ts

Splaktar · 2019-11-08T01:12:20Z

src/google-maps/map-polyline/map-polyline.spec.ts

+                              (polylineClick)="handleClick()"
+                              (polylineRightclick)="handleRightclick()">
+                </map-polyline>
+            </google-map>`,


Not a big deal, but it seems like this template is 4 space indented vs 2.

I think this is something unusual with how spacing in multi line template literals are being interpreted. It is two spaces on my IDE, I'm not sure if there's a setting I need to to change on my IDE or what but I'll look into it.

src/dev-app/google-map/google-map-demo.html

Add a MapPolyline component to @angular/google-maps that allows drawing polylines on a Google Map.

Clean up test data and use underscore for private members.

Change component to a directive.

Fix lint error due to exceeding line length limit.

Change event listener prefix from "map" to "polyline" in polyline component to indicate the different behavior of the polyline events.

Clean up demo component.

Splaktar · 2019-11-09T01:39:36Z

As mentioned in the api_golden_checks build message you need to run a local Bazel command to approve the changes to the public API.

This can be done via bazel run //tools/public_api_guard:google-maps/google-maps.d.ts_api.accept.

Then you can amend your commit and force push it up to your branch.

crisbeto · 2019-11-09T01:42:33Z

@Splaktar the --define=compile=legacy shouldn't be added anymore. Now we're generating the API goldens with Ivy.

Splaktar · 2019-11-09T01:44:25Z

@crisbeto Oh good! I'll update it.

@mbehrlich Then it would be good if you could squash your commits. This guide has instructions for that. In the future, if you amend your commit and force push, you can avoid the need to squash.

Splaktar

Other than the need to squash and update the API Goldens, this LGTM!

jelbourn · 2019-11-09T18:22:55Z

No need to manually squash commits any more; we always just use "Squash and merge" on github

(though I tend to always squash mine anyway out of habit)

Add Polyline to public API.

mbehrlich · 2019-11-13T15:54:30Z

Should be ready now.

jelbourn · 2019-11-18T16:49:03Z

Caretaker note: can be merged after 9.0.0 is released

angular-automatic-lock-bot · 2020-01-03T00:05:25Z

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.}

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 26, 2019

crisbeto reviewed Oct 26, 2019

View reviewed changes

mbehrlich requested a review from crisbeto November 1, 2019 18:11

jelbourn reviewed Nov 1, 2019

View reviewed changes

src/google-maps/map-polyline/map-polyline.ts Outdated Show resolved Hide resolved

src/google-maps/map-polyline/map-polyline.ts Outdated Show resolved Hide resolved

mbehrlich requested a review from jelbourn November 5, 2019 01:55

jelbourn added the target: minor This PR is targeted for the next minor release label Nov 6, 2019

mbehrlich requested a review from Splaktar November 7, 2019 23:40

Splaktar reviewed Nov 8, 2019

View reviewed changes

mbehrlich added 6 commits November 8, 2019 12:15

feat(google-maps): Add MapPolyline component

767ca50

Add a MapPolyline component to @angular/google-maps that allows drawing polylines on a Google Map.

feat(google-maps): Add MapPolyline component

3774940

Clean up test data and use underscore for private members.

feat(google-maps): Add MapPolyline component

3488a67

Change component to a directive.

feat(google-maps): Add MapPolyline component

555f64c

Fix lint error due to exceeding line length limit.

feat(google-maps): Add MapPolyline component

4c882c1

Change event listener prefix from "map" to "polyline" in polyline component to indicate the different behavior of the polyline events.

feat(google-maps): Add MapPolyline component

3a84118

Clean up demo component.

mbehrlich force-pushed the map-polyline branch from ad10e7f to 3a84118 Compare November 8, 2019 21:40

mbehrlich requested a review from Splaktar November 8, 2019 23:40

Splaktar approved these changes Nov 9, 2019

View reviewed changes

feat(google-maps): Add MapPolyline component

9e7f3a8

Add Polyline to public API.

mbehrlich requested a review from a team as a code owner November 11, 2019 20:55

Splaktar added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Nov 14, 2019

jelbourn added pr: merge safe and removed blocked This issue is blocked by some external factor, such as a prerequisite PR labels Dec 3, 2019

jelbourn merged commit 0c10828 into angular:master Dec 3, 2019

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(google-maps): Add MapPolyline component #17512

feat(google-maps): Add MapPolyline component #17512

mbehrlich commented Oct 26, 2019

crisbeto Oct 26, 2019

mbehrlich Oct 28, 2019

jelbourn left a comment

mbehrlich commented Nov 4, 2019

Splaktar Nov 8, 2019

mbehrlich Nov 8, 2019

Splaktar commented Nov 9, 2019 •

edited

Loading

crisbeto commented Nov 9, 2019

Splaktar commented Nov 9, 2019

Splaktar left a comment

jelbourn commented Nov 9, 2019

mbehrlich commented Nov 13, 2019

jelbourn commented Nov 18, 2019

angular-automatic-lock-bot bot commented Jan 3, 2020

feat(google-maps): Add MapPolyline component #17512

feat(google-maps): Add MapPolyline component #17512

Conversation

mbehrlich commented Oct 26, 2019

crisbeto Oct 26, 2019

Choose a reason for hiding this comment

mbehrlich Oct 28, 2019

Choose a reason for hiding this comment

jelbourn left a comment

Choose a reason for hiding this comment

mbehrlich commented Nov 4, 2019

Splaktar Nov 8, 2019

Choose a reason for hiding this comment

mbehrlich Nov 8, 2019

Choose a reason for hiding this comment

Splaktar commented Nov 9, 2019 • edited Loading

crisbeto commented Nov 9, 2019

Splaktar commented Nov 9, 2019

Splaktar left a comment

Choose a reason for hiding this comment

jelbourn commented Nov 9, 2019

mbehrlich commented Nov 13, 2019

jelbourn commented Nov 18, 2019

angular-automatic-lock-bot bot commented Jan 3, 2020

Splaktar commented Nov 9, 2019 •

edited

Loading