-
-
Notifications
You must be signed in to change notification settings - Fork 860
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
feat: add static Marker.computePixelAlignment
method to calculate absolute alignment
#1847
Conversation
…lignment Impacted files: * `marker.dart`: added `factory Marker.withPixelAlignment` * `markers.dart`: now a click on the map creates a new "U-turn-left" marker with custom alignment
Marker.withPixelAlignment
with left+top custom alignment
This is a good point, and it makes me wonder whether it would be better just to have a more simple implementation where |
Looks good to me! I've just pushed it. |
@JaffaKetchup Finally I've removed my inconsistent example and just kept the new static |
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.
I personally liked the factory constructor better since it was more self explaining to me.
Seeing that the relative alignment gets calculated into top/left pixel values anyways I wonder if it would be better to make pixel alignment the default and give users the option for relative alignment?
But since non-realtive marker alignment is a much needed feature so I'm down for a static method.
Edit: It would be really helpful to hint to this static method and its usage from the default Marker constructor.
I agree with you. It can still be done, in this PR or a next one.
Possible, but I'd like to avoid breaking changes.
Cool
I've just added /// [alignment] may be computed using [computePixelAlignment]. I don't know how it works here: feel free to merge my PR when relevant. |
Marker.withPixelAlignment
with left+top custom alignmentMarker.computePixelAlignment
method to calculate left+top alignment
Marker.computePixelAlignment
method to calculate left+top alignmentMarker.computePixelAlignment
method to calculate absolute alignment
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.
LGTM, thanks for this :)
I think it's good that we've distilled it down into the most vital component, we can always make the factories at a later point, as you said.
What I've done:
factory Marker.withPixelAlignment
in order to set pixel-precise marker alignment, cf. [FEATURE] (Re-)Support absoluteMarker
alignment #1771What can still be done:
factory
- but I wanted to keep theconst
and I'm not sure we can do that if computing the alignmentMarker
has a specific pixel-alignment constructor thatMarkerLayer
does not have. Not sure if that would make sense to create a newMarkerLayer
constructor with the left+top parameters being used as default.Impacted files:
marker.dart
: addedfactory Marker.withPixelAlignment
markers.dart
: now a click on the map creates a new "U-turn-left" marker with custom alignment