Use CSS filter instead of dimming tiles

[scy]

Description

This is basically a hotfix to address the dark mode rollout from today. I'm new to contributing here and kind of in a hurry, sorry in advance if anything in this PR isn't perfect.

The current implementation of dark mode is already pretty good, but the tile display (i.e. the actual map) suffers greatly, since the tiles are simply dimmed using a CSS filter, losing a lot of contrast in the process.

I have borrowed the filter designed by @krjan02 from https://github.com/openstreetmap/openstreetmap-website/issues/2332#issuecomment-867821340 and basically inserted it into the changes made in #4712.

Additionally, I have made sure that the tiles are not being filtered twice due to inheriting the filter from their parent element. Maybe selecting .leaflet-tile instead of .leaflet-tile-container would solve that problem, too, but I'm assuming that @AntonKhorev had a reason for targeting the container instead.

This is how it would look:

Note that the discussion in #2332 is apparently not over yet, and I don't want to preempt this. However, since dark mode rolled out today and I've already seen several people kind of unhappy with the way it looks now (which is made worse by the fact that there's no way to turn it off if your browser is set to prefer dark mode), I wanted to suggest a quick fix.

How has this been tested?

That's the thing: I have tested this by editing the CSS in the browser developer tools, because I don't want to set up the whole environment for building/running the website locally. As I said, I'm new to this project. I'd appreciate if someone with a working setup could test the changes.

However, as you can see in the screenshot, the map key is correctly affected by the filter, as are the previews in the "Map Layers" pane.

Also, I'm an experienced developer, but not a front-end dev.

[tomFlowee]

For what it is worth, as someone that loves dark mode and even has an AMOLED display that makes a pure black background very easy on the eyes, for someone that is a fan of dark mode, I don't want my normal map usage to have tiles that have a dark background.

Only while driving I like having dark background with white street/city names, for a map that has very low amount of detail. But for normal usage, which is 100% of what I'd do on OSm.org, I don't want my tiles to be anything other than the beautiful color-theme that the site has shipped for years.

This "feauture" should at minimum be opt-in.

[scy]

This "feauture" should at minimum be opt-in.

I agree, but that is out of scope for this issue. It would need JavaScript changes (basically the code from the Bootstrap docs) and an additional UI element. That's no longer a quick fix.

(Also, please refrain from copy-pasting comments from other issues.)

[RedAuburn]

how does it look with the other map styles?

[tomhughes]

We're not going to be merging anything in a hurry anyway so talk of a "hotfix" or of not having time to do things properly is not helpful.

If changes are need we should get them right rather than keep cycling through multiple changes.

Obviously as the person that merged the original change here this is partly down to me but dark mode is not something I use or have any experience with so I had little to go on other than my own experience where I find having bright things in the middle of a dark screen really odd and hard to look at which is why I went with what appeared to be the best available option to get something that, while not ideal, was the least worst option for people that insisted on using dark mod.

[tomhughes]

Having not looked at what is being proposed here my concern is exactly what I think was discussed originally, that I think this is completely distorting the colours chosen by the style designer? Which we were concerned was not a reasonable thing to and something that might be received badly by style designers.

[scy]

how does it look with the other map styles?

And before you say "but that's not perfect": You might be right, but it's better than the dimming. :wink:

[tomhughes]

This "feauture" should at minimum be opt-in.

That is something to disuss at #5324 not here.

I agree, but that is out of scope for this issue. It would need JavaScript changes (basically the code from the Bootstrap docs) and an additional UI element. That's no longer a quick fix.

No, it should probably done as a setting on the users settings page, not with a lot of client side javascript.

[scy]

We're not going to be merging anything in a hurry anyway so talk of a "hotfix" or of not having time to do things properly is not helpful.

In that case, please revert the dark mode, and I'll close this PR.

I'm proposing this change as a service to the community, with multiple people having called the page "unusable" now (and I have to say that I agree). And I'm not interested in spending an hour or more to set up a dev environment. Feel free to take the change, or modify it, or revert, or don't do anything at all. I'm fine with all of that. I'm not fine with spending hours now to defend this proposal.

If changes are need we should get them right rather than keep cycling through multiple changes.

I'm sorry if I misjudged the amount of testing and feedback that goes into a change like this. That assumption was based on the "dark mode" that was deployed without regarding, or notifying, the people working on it in #2332, and it was released in a state that's irritating and confusing users and seriously breaking accessibility.

Again: If you don't agree with these changes, don't take them, and revert instead, or simply remove the dimming and have the "bright" tiles displayed and just the rest of the UI be dark. Or don't do anything, it's not my place to tell you what to do. I'm just proposing a change here.

Obviously as the person that merged the original change here this is partly down to me but dark mode is not something I use or have any experience with so I had little to go on other than my own experience where I find having bright things in the middle of a dark screen really odd and hard to look at which is why I went with what appeared to be the best available option to get something that, while not ideal, was the least worst option for people that insisted on using dark mod.

With all due respect, but if you're not even using dark mode, you might not be the best person to decide what's "the least worst option".

Because right now, dark mode users have to deal with the map looking like there's an invisible popup somewhere that's dimming everything, and they can't even revert to looking at OSM in "light mode" without changing their whole browser to light mode, for all websites.

my concern is exactly what I think was discussed originally, that I think this is completely distorting the colours chosen by the style designer? Which we were concerned was not a reasonable thing to and something that might be received badly by style designers.

Obliterating the contrast on the existing design is also "completely distorting the colours".

If you want to keep the colors as they are, remove the dimming and leave the map bright as it was before. As I said, I'm fine with that, and I think that most, if not all, dark mode users would still prefer that to the dimmed tiles.

(However, as a dark mode user, I was blown away by the designs in #2332 and would prefer any of them to a bright map, since, as you said, being blinded in the dark by tiles not designed for dark mode sucks.)

[tomhughes]

There were dozens of pull requests over many months so it was hardly done in a hurry beyond maybe the final merge to turn it on which I admit took me somewhat by surprise.

I'll leave an decision about reverting that decision to the person that made it and and will bow out of all future discussion about dark mode.

[Dr-Mx]

I quite like the new dark mode. I've used that inversion filter OP mentioned on my own website, but wasn't a fan due to the terrible performance on old devices.

I think adding contrast(1.1) to the filter would help readability.

[mxdanger]

This should be discussed in depth and not treated as a "quick fix". Theres serious implications to changing the color of a map that has been meticulously colored and iterated on for the past decade. Realistically, I don't expect a "default" dark map theme until the vector map is released.

However, for the time being, the existing dimming filter on the map should absolutely be removed as a quick fix.

[AntonKhorev]

Additionally, I have made sure that the tiles are not being filtered twice due to inheriting the filter from their parent element. Maybe selecting .leaflet-tile instead of .leaflet-tile-container would solve that problem, too, but I'm assuming that @AntonKhorev had a reason for targeting the container instead.

I wrote all of that months ago and I don't remember what things I did for what reason. filter is normally not inherited but Leaflet sets it to inherit for tiles. If I target .leaflet-tile, that filter: inherit overrides my filter declaration, that's probably why I targeted something else without checking if the filter is applied twice. Applying twice wouldn't work for inversion, that's why it got noticed here. But I also had another implementation #4777 with working inversion.

[AntonKhorev]

They stopped inheriting filter in Leaflet but that didn't make it yet to releases https://github.com/Leaflet/Leaflet/pull/8651. I guess it's safe to override their inherit.

[pkrasicki]

Alternative proposal:

filter: brightness(0.6) invert(1) contrast(3) hue-rotate(200deg) saturate(0.3) brightness(0.7);

You can test it here: https://issviewer.com Full source code: https://github.com/pkrasicki/issviewer

[scy]

It seems pretty clear to me that this PR is not going to get merged. Instead, #5330 is being fast-tracked, because "making it a little less dim" is assumed to clearly fix everyone's problem with the changes.

Meanwhile, #2332 is being closed, with the suggestion being that people just make their suggestions again in new issues.

There are little to no comments being made in this PR by the maintainers as to what needs to change for it to get merged, or whether there's any chance of it getting merged in the future, or whether there are plans to revert the failed dark mode, or whether there's at least someone from the team working on it who actually uses dark mode.

Please try to understand my point of view as an external contributor when I say: I'm not under the impression that contributions or concerns by non-team people are being appreciated. We're left to discuss stuff for years, and then some embarrassing "just make it less bright" is being merged and rolled out without any discussion or consultation whatsoever.

I for one have decided to just fix it with a userscript and not bother trying to upstream any improvements, or contribute to the website going forward.

[gravitystorm]

because "making it a little less dim" is assumed to clearly fix everyone's problem with the changes.

That's not why it was merged, and I think if you read #5328 you'll see that I'm not "assuming that it's going to fix everyone's problems". That's a pretty unfair characterisation.

There are little to no comments being made in this PR by the maintainers as to what needs to change for it to get merged

It's been less than 22 hours since you opened it. I haven't reviewed this PR specifically, but you can see from the discussion in #5328 that there's lots to consider about the invert+rotate approach. It's not a PR which is a straightforward merge, and even if it was, 22 hours is asking a lot in terms of turnaround from a handful of volunteers.

whether there are plans to revert the failed dark mode

I think "failed" is also unfair. There's more to dark mode than just effect on the maps, it took months of work and dozens of changes across that rest of the site too. That's why we are discussing things in separate issues. Even today we've been making improvements to other aspects of the dark mode UI changes, which are unrelated to the maps.

without any discussion or consultation whatsoever

4769 was opened in May, but unfortunately there was no substantial discussion.

not bother trying to upstream any improvements, or contribute to the website going forward.

That's a shame, we do welcome everyone to get involved. I've even given conference talks about what we do to make things easier (e.g. 1 2). You can also see the endless list of PRs we make with the Developer Experience (dx) tag. I know it's a long way from perfect, but we're a small team with a lot of work to do.

[scy]

Let me start by saying that yes, I'm disappointed, but I don't mean to hate. I understand that you're all busy, and that this is volunteer work. I'm not expecting anything from you. Please read my comments more as feedback on how I feel and how things are looking like from my end, written in the hope that the experience might improve for others in the future.

That's not why it was merged, and I think if you read #5328 you'll see that I'm not "assuming that it's going to fix everyone's problems". That's a pretty unfair characterisation.

AFAICT, #5328 is not talking about #5330 at all. I don't understand how "let's keep dimming the tiles, but just a bit less" is treated as a valid solution to the backlash about the "dark mode" that rolled out yesterday.

And how was that selected as the way to go in the first place? Because #2332 sure doesn't seem to favor it. Instead, there was PR #4712 that brought us here in the first place, merged with no discussion whatsoever, and without pinging any of the existing issues. This feels like "yeah whatever, keep talking, we're gonna do this my way now". And that's massively off-putting.

It's been less than 22 hours since you opened it. I haven't reviewed this PR specifically, but you can see from the discussion in #5328 that there's lots to consider about the invert+rotate approach. It's not a PR which is a straightforward merge, and even if it was, 22 hours is asking a lot in terms of turnaround from a handful of volunteers.

If this were a feature request, or even a normal bugfix, sure, I understand your argument. But from my perspective, the deployment yesterday broke the map for almost every dark mode user, with basically no option to get back the previous behavior. The only way people could continue using the OSM.org map was either by disabling dark mode for their whole browser (because that's a global setting both in Firefox and Chrome), or use a different browser just for looking at OSM.

That's something that, in my book, should either be hotfixed or rolled back ASAP. And it's also something that wouldn't have happened in the first place if the whole "no we need to weigh the options against each other and thoroughly test and discuss this" attitude would've been applied before rolling out the change, and it feels unfair to now insist on it when the broken release was already deployed.

I think "failed" is also unfair. There's more to dark mode than just effect on the maps, it took months of work and dozens of changes across that rest of the site too.

And I congratulate you on this. Because I know that it's not easy, and it looked really good. I even acknowledged it at the top of this PR ("The current implementation of dark mode is already pretty good").

But you'll also have to admit that breaking the map for a lot of users is a huge deal. I don't see how the dimmed tiles would've been approved by anyone who cares about accessibility, or uses dark mode. Therefore I have to conclude that it's not been tested or reviewed by the actual stakeholders before going live, and it made things significantly worse for them. I consider that a failure.

That's okay, because failures happen, and then we either fix things or roll the changes back, but instead you're twiddling on some percentages and assume that it's gonna fix the issue, and that's just disheartening.

4769 was opened in May, but unfortunately there was no substantial discussion.

The lack of discussion might be related to #2332, the issue titled "Dark Mode", opened before the pandemic, the one where people had been suggesting things for years, not being made aware of it.

That the issue has been closed today, with new ones being created for all the different aspects, might make it easier from a project management perspective to keep track on the different topics. But please understand that for the people who have been waiting for five years to see progress on the issue, and now being presented with a half-baked non-solution without their involvement, it probably feels like a joke.

not bother trying to upstream any improvements, or contribute to the website going forward.

That's a shame, we do welcome everyone to get involved.

And I really appreciate you taking the time to address the points I've brought up. I hope that this reply is helpful to you, too.

[pkrasicki]

1. Have a problem, i.e. lack of dark mode.
2. The proper solution is difficult.
3. People propose temporary workarounds and submit PRs to fix the problem.
4. Reject their PRs and ignore their proposals, listing an infinite amount of side problems that would need to be considered first.
5. The discussion continues for years while users keep complaining about the main issue.
6. Finally implement something that isn't perfect either and ignores people's suggestions.
7. People complain.
8. Close the GitHub issue, but open new ones that talk about side problems and ignore the specific proposals people have made over the years.
9. People complain about that, so tell them they're being dismissive of the depth of the problem.
10. Continue discussing side problems while being as vague as possible, so that you can keep going in circles (bikeshedding).
11. Dismiss any criticism of this process while claiming that you welcome anyone to get involved.

Just look at the current discussion in #5328 where people discuss non existent performance issues, question the need for a dark map in dark theme (we've already explained why we need it...), compare a map application to an image viewer and are talking something about contrast and ink on paper (???). They discuss inverting the colors and how that's bad, because they don't even understand the proposals that were made. They literally think we want to just invert the colors and that's it.

[tomFlowee]

They literally think we want to just invert the colors and that's it.

Actually OSM just darkens the tiles and that's it.

Anyway; you state that

question the need for a dark map in dark theme (we've already explained why we need it...)

Can you link to that explanation? I have doubt.

[matkoniecz]

Instead, https://github.com/openstreetmap/openstreetmap-website/pull/5330 is being fast-tracked, because "making it a little less dim" is assumed to clearly fix everyone's problem with the changes.

based on PR description, it was fast tracked as a bug fix ("darker than I intended them to be")

changes to previous PR made by its author with description "ops, I intended different effect, it is a bug" tend to be fast tracked in general

also, based on https://github.com/openstreetmap/openstreetmap-website/pull/5336#issuecomment-2480560862 they do not expect it to "fix everyone's problem with the changes."

[matkoniecz]

The only way people could continue using the OSM.org map was either by disabling dark mode for their whole browser (because that's a global setting both in Firefox and Chrome), or use a different browser just for looking at OSM

AFAIK map was still shown? At most it was really ugly/poorly readable. I would reserve claims "The only way people could continue" for actually fully broken site

I don't see how the dimmed tiles would've been approved by anyone who cares about accessibility, or uses dark mode

people have different preferences, I for example avoid dark mode whenever possible but I am not going to write how anyone adding dark mode hates users and accessibility

It's been less than 22 hours since you opened it. I haven't reviewed this PR specifically, but you can see from the discussion in https://github.com/openstreetmap/openstreetmap-website/issues/5328 that there's lots to consider about the invert+rotate approach. It's not a PR which is a straightforward merge, and even if it was, 22 hours is asking a lot in terms of turnaround from a handful of volunteers.

+1

it is not really reasonable to expect PR review within 24 hours

for open source maintained by volunteers I would not expect it even for critical issues breaking critical systems (if that happens it is fault of anyone supposed to provide funding for these systems, and in this this case it was not actually making site unusable even if it was widely hated)

You have right to expect PR review within 24 hours if you have people paid specifically to do so.

I would advise to not rage-quit over this, openstreetmap-website would likely benefit from more developers (though there is also bottleneck at PR reviewing)