Remove brightness filter for map tiles in dark mode styles

openstreetmap / openstreetmap-website

The Rails application that powers OpenStreetMap

https://www.openstreetmap.org/

GNU General Public License v2.0

2.21k stars 918 forks source link

Remove brightness filter for map tiles in dark mode styles #5327

Open sammyhawkrad opened 1 week ago

sammyhawkrad commented 1 week ago

Description

It's nice to finally have a dark mode on the website but reducing the brightness of the default map tiles is not ideal and negatively affects the user experience. This PR removes the filter on the map tiles to keep the default view of the map.

Sets the dark mode view to this:

from this:

I also tested the brightness at .95 which is quite bearable if the dimming is necessary.

mxdanger commented 6 days ago

Glad to see this change! It's a shame that @AntonKhorev saw all the feedback about the dimmed map and completely ignored it with fast tracking this change #5330 instead of removing the filter all together until #5336 is completed.

AntonKhorev commented 6 days ago

It's a shame that @AntonKhorev saw all the feedback

I saw that there's a bug.

and completely ignored it with fast tracking this change https://github.com/openstreetmap/openstreetmap-website/pull/5330

This is a fix for that bug.

until https://github.com/openstreetmap/openstreetmap-website/pull/5336 is completed

That's not what necessarily will be deployed.

Nekzuris commented 4 days ago

What are you waiting to merge this?

ChrisJoubert2501 commented 4 days ago

What are you waiting to merge this?

This PR needs to be updated, because another PR was recently merged that alters the brightness filter.

tomFlowee commented 4 days ago

Can someone please rebase and merge this?

Its monday, plenty of office-going people will see the darkened map for the first time and it would help immensely to avoid the influx of bugreports and complaints on support channels.

We know from the large amount of upheaval that this css rule brought that this is not the right solution, so I'm not making this up. We've only seen the volunteer's reaction so far, not the big crowds.

Thanks!

github-actions[bot] commented 2 days ago

	1 Warning
:warning:	Merge commits are found in PR. Please rebase to get rid of the merge commits in this PR, see CONTRIBUTING.md.

Generated by :no_entry_sign: Danger

matkoniecz commented 2 days ago

What are you waiting to merge this?

Maybe for discussion in #5328 to conclude? Reacting to comments there or posting new, if you have something new to say, may be a good idea.

mxdanger commented 1 day ago

Maybe for discussion in #5328 to conclude?

@matkoniecz The problem is that the map should not have been dimmed to begin with, especially without any proper discussion. This should be merged and the map should remain as default until #5328 is concluded.