-
Notifications
You must be signed in to change notification settings - Fork 0
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
break: condense and deprecate redundant styling-related props #473
Conversation
✅ Deploy Preview for oslmap ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -88,6 +88,9 @@ export class MyMap extends LitElement { | |||
@property({ type: String }) | |||
drawType: DrawTypeEnum = "Polygon"; | |||
|
|||
/** | |||
* @deprecated - please set `drawColor` regardless of `drawType` | |||
*/ |
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.
My thinking is we'll do a NPM prerelease
with the @deprecated
annotations, then fully remove them ahead of the stable release - sound like a decent plan?
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.
Perfect!
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.
Super helpful and clear PR description thanks 🙌
let featureColor = feature.get("color") || this.geojsonColor; // Use the geojsonColor if no color property exists | ||
|
||
// Read color from geojson feature `properties` if set or fallback to prop | ||
let featureColor = feature.get("color") || this.geojsonColor; |
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.
nit: It looks like this could actually be a const
?
Another "pre v1" task might be to set up linting here to catch a few of these type of things?
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.
Yep great shout - will handle in a followup PR 👍
Our styling-related props haven't always been so consistent, so let's tidy them up with a v1 release in mind & flag some as deprecated.
Here's a quick overview of exisitng & changing style-related props based on various modes/functionalities:
drawColor
(string) - fill automatically applied at 20% (used to be 10%)now deprecated - it's only possible to set a singledrawPointColor
(string)drawType
at a time anyways, sodrawColor
is sufficientnow deprecateddrawFillColor
(string)drawPointer
(enum)markerColor
(string)markerImage
(enum)geojsonColor
(string)geojsonFill
(boolean) - 20% if truefeatureColor
(string)featureFill
(boolean) - 20% if truenow deprecatedfeatureBorderNone
(string)Any other votes for deprecation or refactoring?