Skip to content

Switch to inline colourable markers #5854

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

hlfan
Copy link
Contributor

@hlfan hlfan commented Mar 27, 2025

Description

This is an attempt at parametrifying marker colors with currentColor since it has come up in #5764.
Together with optimizing the path geometry, this required rewriting the marker layout to be compatible with partially transparent gradients.
While an even simpler path has been proposed and demonstrated in https://antonkhorev.github.io/msvgen, I was searching for a solution without a sudden change in curvature, which I found by constraining a Bezier curve to a specified slope, 1.92 in this case.1 (See CodePen)

Where I had the source colors, I tried to fit the colors to the sources, e.g. for the note markers see this before/after:
image

For now, this is only a barebones switch to the new markers, but moving in the direction of #872 (comment), #2787 or #507

Closes #5764.

How has this been tested?

Server side and client/browser side separately, no integration testing though.

Considerations

For the raster icons where I didn't have the colors, I took them from the note markers, so

  • either I need better sources than I could find, or
  • the dot markers will differ in color a bit more:
    image

Footnotes

  1. Leaflet's marker.svg actually has two slopes for the tip, 1.9205 on the left and 1.9296 on the right

@hlfan hlfan marked this pull request as draft March 28, 2025 06:28
@hlfan hlfan force-pushed the colorable-markers branch 2 times, most recently from 20f1ebb to 20e655b Compare March 31, 2025 12:25
@hlfan hlfan marked this pull request as ready for review March 31, 2025 12:25
@hlfan hlfan force-pushed the colorable-markers branch 2 times, most recently from efa51b2 to cb0e3d3 Compare March 31, 2025 13:43
@hlfan hlfan force-pushed the colorable-markers branch 2 times, most recently from 485c74e to 7806af3 Compare April 17, 2025 00:00
@hlfan hlfan mentioned this pull request Apr 20, 2025
@hlfan hlfan force-pushed the colorable-markers branch 2 times, most recently from 322a722 to 8bb6564 Compare May 2, 2025 01:48
@hlfan hlfan force-pushed the colorable-markers branch from 8bb6564 to e3c859d Compare May 2, 2025 18:37
@hlfan hlfan force-pushed the colorable-markers branch from e3c859d to 2d75db9 Compare May 2, 2025 22:45
@tomhughes tomhughes requested a review from AntonKhorev May 3, 2025 13:54
@hlfan
Copy link
Contributor Author

hlfan commented May 3, 2025

@tomhughes do you think it's better to match the current marker colors more closely or to reduce variations in color (only have one green, one red...)?

@tomhughes
Copy link
Member

I couldn't see that much of a difference to be honest - there was a bit of one with the routing markers but nothing terrible.

I know this overlaps with work @AntonKhorev has been doing though which is why I've asked him to review it.

@tomhughes
Copy link
Member

The only other thing that struck me was the various hex colour constants scattered around but I'm not sure there's any good way to centralise them.

@hlfan hlfan force-pushed the colorable-markers branch from 2d75db9 to 4c5433f Compare May 3, 2025 17:15
@hlfan hlfan force-pushed the colorable-markers branch from 4c5433f to 0d339d4 Compare May 3, 2025 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants