feat: add support for drawing and modifying Circle geometry types#465
feat: add support for drawing and modifying Circle geometry types#465jessicamcinchak merged 1 commit intomainfrom
Circle geometry types#465Conversation
✅ Deploy Preview for oslmap ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
RODO94
left a comment
RODO94
left a comment
There was a problem hiding this comment.
Looks good from what I can see and test, only one wee nit.
| return new Style({ | ||
| image: new Circle({ | ||
| radius: 9, | ||
| fill: new Fill({ color: pointColor }), |
There was a problem hiding this comment.
nit: are pointColor and drawColor controlling similar things but relate semantically to different parts. Point referring to the color of your Pointer compared with drawColor which refers to the colour of polygons and shapes which you draw?
Could be cleaner to consolidate them into drawColor unless they are controlling different things.
There was a problem hiding this comment.
Yep totally agree it'd be nice to consolidate everything into drawColor !
This will technically constitute a breaking change though from the current release - so I vote we handle this in a separate PR where we can adjust behavior and consider marking existing props as @deprecated for upcoming release before straight up removing?
2 participants
Replaces #464 - let's introduce one new shape at a time !
Changes:
drawTypeprop now supportsCirclein addition to originalPolygon(still default) andPointdrawTypeternaries to switch statements, which will scale a lot nicer as we support more draw types going forward (for now, the circle is sharing stroke and fill styles with the polygon)Followup PRs:
drawType="Circle"fromCircle()here, but getting some type errors (eg our drawn circle is type "SimplifiedCircle" geometry rather than true "Circle" that will be easier to troubleshoot in a separate followup!)