-
Notifications
You must be signed in to change notification settings - Fork 923
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
Use map tiles dark mode with leaflet-osm plugin #5396
base: master
Are you sure you want to change the base?
Conversation
This follows the logic from openstreetmap#5328
As requested in openstreetmap#5395.
This being done here eliminates the need to specify `filter: none` in every tile layer definition.
Changes to anything under |
Also inline styles are not allowed by our security policy in production so any dynamic changes need to be done by adding or removing classes to trigger static style rules in the main stylesheet. |
I wrote it's a leaner variant of #4777, not that I fixed its flaws.
That's why I left openstreetmap/leaflet-osm#42 open.
The |
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 don't think leaflet-osm should be getting involved in automatic transforms - it should have the ability to select a dark tile version where there is one and any automatic transformations should only be in the osm.org code.
options = L.Util.setOptions(this, options); | ||
L.TileLayer.prototype.initialize.call(this, options.url); | ||
url = isDarkMap ? options.darkUrl : options.lightUrl; |
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.
There is no lightUrl
anywhere?
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.
Well, if the provided layers can be expected always to be a light-mode-first choice, the logic can be simplified to this more unreadable version:
options.filter = isDarkMap && options.darkFilter || 'none';
options = L.Util.setOptions(this, options);
url = isDarkMap && options.darkUrl;
this.schemeClass = url && 'dark';
But I thought an implementation agnostic to that would be easier to think through with more options for cartographers' choices.
OK so it is allowed for views with a map. I'm note sure why that is offhand but our goal is to not have it so we don't add new inline styles ourselves but we sometimes have to allow it for third party components. |
Do you mean only the extension of |
This lean and updated variant of #4777 follows the rules laid out in #5328 that were implemented in #5362.
Since the editing of the user preference happens on a different view/site, there is no need to listen to color scheme changes on the fly and the whole logic can happen in the initialization of
L.OSM.TileLayer
.Additionally, a "dark" CSS class is introduced to allow better style targeting if the user wants to.
for example like this: