@gravitystorm commented on this pull request.
Great, thanks for the PR. I want to make sure that we're fixing this the right
way.
This PR introduces a colour override for the text in the context menu, in light
mode only. We currently override the background colour in both the light mode
and the dark mode. Also we are specifying the background colour using SCSS
variables, but the text colour in CSS variables.
The underlying cause of the bug is that we are overriding the background colour
in the `hover` and `active` states, but bootstrap also has code to override the
background colour and also the foreground colour in each of those states. By
default it adds a small amount of background shading (and no text colour
changes) in hover, and then a primary background colour (blue) and suitable
text colour (currently white) in the active state.
I'm not really sure why we override anything to begin with. One solution is to
remove the background colour overrides (x2) and then the menu behaves as a
standard bootstrap menu. The advantage here is that if we (or Bootstrap) change
things like the overall bootstrap colour scheme, then the menu will
automatically work as intended. Additionally, if we use more bootstrap dropdown
menus elsewhere, they will all behave the same way.
Alternatively, if we want to retain these colour overrides, I would rather see
the background and foreground both linked to the relevant link colours, either
SCSS or CSS but not a mixture of both.
Finally, we already use the context menu in one other place (the user menu, top
right) and we should at least aim to be consistent between them!
What do you think?
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6542#pullrequestreview-3483242388
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/6542/review/[email protected]>
_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev
