-
Notifications
You must be signed in to change notification settings - Fork 88
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
Use AdvancedMarkerElements and add index to markers on map #2109
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @amysorto! Lgtm, one small code health tax if you have time.
@rfontanarosa for final approval.
lois | ||
.filter(loi => | ||
this.showPredefinedLoisOnly ? loi.predefined : true | ||
]).subscribe( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is becoming unwieldy. While you're here any chance you could refactor it a bit, perhaps pulling out meaningful logic into helper functions?
c026666
to
509b96d
Compare
@amysorto Please ping when RFAL! |
@gino-m This PR is ready for review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥳
Part of #1477. Adds annotations for submission markers (but not polygons). @gino-m As far as I know we don't have polygons yet in our submissions. Should this PR close this issue instead and we file a separate issues for polygons?
I also filed #2108 to use custom HTML and CSS for the markers. When I initially tried the styles were not being correctly applied so I figured I can take a look into that separately so we can have better map annotiations in the meanwhile.
Screenshot:
Also I changed the index in the submission panel to start at 1 instead of 0. I thought there was a filed issue for this but I couldn't find it to link it here.