Skip to content

feat(google-maps): Add MapPolygon component #17936

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 2 commits into from
Jan 15, 2020

Conversation

mbehrlich
Copy link
Collaborator

Add MapPolygon component to Google Maps, allowing polygons to be added
to a Google Map.

Add MapPolygon component to Google Maps, allowing polygons to be added
to a Google Map.
@mbehrlich mbehrlich requested a review from a team as a code owner December 11, 2019 18:35
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 11, 2019
@mbehrlich mbehrlich assigned mbehrlich and jelbourn and unassigned mbehrlich Dec 19, 2019
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -28,4 +29,13 @@
<input id="editable-polyline-checkbox" type="checkbox" (click)="toggleEditablePolyline()">
</div>

<div>
<label for="polygon-checkbox">Toggle Polygon</label>
<input id="polygon-checkbox" type="checkbox" (click)="togglePolygonDisplay()">
Copy link
Member

Choose a reason for hiding this comment

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

optional: you can put the input inside of the label to associate them rather than adding IDs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@jelbourn jelbourn added blocked This issue is blocked by some external factor, such as a prerequisite PR pr: lgtm action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Dec 19, 2019
@jelbourn
Copy link
Member

Marking this as blocked since it will need to go in after we release v9

@jelbourn jelbourn removed pr: lgtm action: merge The PR is ready for merge by the caretaker labels Dec 19, 2019
@jelbourn
Copy link
Member

@mbehrlich actually I tried the demo locally and there seems to be an error when enabling the polygon:

core.umd.js:4194 ERROR TypeError: Cannot read property 'getPaths' of undefined

@mbehrlich
Copy link
Collaborator Author

Does this still need to be blocked since it's being merged into google-maps-next and not master?

@mbehrlich
Copy link
Collaborator Author

mbehrlich commented Dec 20, 2019

@jelbourn The type error occurs if you click "toggle editable polygon" when the polygon itself is not displayed. As far as @angular/google-maps is concerned, that's working as intended. To fix this in the demo, I've disabled the editable toggle when the polygon (or polyline) itself is not being displayed.

Fix issues with checkboxes for demo app for MapPolygon.
@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed blocked This issue is blocked by some external factor, such as a prerequisite PR labels Jan 15, 2020
@jelbourn jelbourn merged commit 8b8f951 into angular:google-maps-next Jan 15, 2020
mbehrlich added a commit to mbehrlich/components that referenced this pull request Jan 31, 2020
Add MapPolygon component to Google Maps, allowing polygons to be added
to a Google Map.
jelbourn pushed a commit that referenced this pull request Feb 13, 2020
Add MapPolygon component to Google Maps, allowing polygons to be added
to a Google Map.
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants