Skip to content
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 dottedSpacingFactor to customize dotted polyline spacing #1845

Merged
merged 6 commits into from
Mar 12, 2024
Merged

feat: add dottedSpacingFactor to customize dotted polyline spacing #1845

merged 6 commits into from
Mar 12, 2024

Conversation

gnassro
Copy link
Contributor

@gnassro gnassro commented Mar 7, 2024

Now we can customize the space between dots if isDotted=true

@josxha josxha changed the title add dottedSpacingFactor to customize dotted polyline spacing feat: add dottedSpacingFactor to customize dotted polyline spacing Mar 8, 2024
@josxha
Copy link
Contributor

josxha commented Mar 8, 2024

Thanks for the pull request @gnassro!
I did a quick test and your changes seem to work well.

There are some small things I would prefer to have changed:

  • We have an open feature request for dashed polylines. So it wood be good to have the name a bit more general like "segmentSpacingFactor" or somthing similar. Then we can use the parameter for dashed polyine too once the feature arrives.
  • The CI checks show that the code is not formatted, you can do this with dart format ., but if you have difficulties I can do it for you if you want. (edit: a trailing comma at the constructor parameters should work too)

Then there are some general things I discovered while testing that you don't need to care about but are worth writing down:

  • changing isDotted doesn't get picked up by hot reload
  • It would be in general nice to have the dotted Polyline features represented in the example app, but I think that putting all on the polyline page would look cluttered.

@josxha josxha added this to the v7.0 milestone Mar 8, 2024
@josxha josxha added the S: core Scoped to the core flutter_map functionality label Mar 8, 2024
@josxha josxha self-requested a review March 8, 2024 13:54
@gnassro
Copy link
Contributor Author

gnassro commented Mar 9, 2024

@josxha the only thing that I didn't understand is, what do you mean by changing isDotted doesn't get picked up by hot reload ?

Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

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

LGTM, thanks :). Will wait for a second review before merging.

@JaffaKetchup JaffaKetchup removed the S: core Scoped to the core flutter_map functionality label Mar 10, 2024
@josxha
Copy link
Contributor

josxha commented Mar 12, 2024

Sorry for the delay, changes looking good. Thanks for updating the example app too.

the only thing that I didn't understand is, what do you mean by changing isDotted doesn't get picked up by hot reload ?

This is a bug that is not related to this pull request. If you

  • run the app in debug mode
  • change isDotted: true, to isDotted: false,
  • trigger a hot reload

Then the polyline doesn't change it's appearance from a dotted to a normal line. A quick guess is because the polyline hashCode gets cached it doesn't regocnizes the change - but anyways as said it's not related to this pr.

Thanks a lot for the contribution!

@josxha josxha merged commit a36c59a into fleaflet:master Mar 12, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants