Miscellaneous white background fixes #162
Conversation
jason-wihardja
changed the title
| .mwe-popups-extract .mwe-popups-fade, | ||
| .mw-mmv-permission-box .mw-mmv-permission-text .mw-mmv-permission-text-fader { | ||
| background-image: linear-gradient(to bottom, transparent, var(--gray-2)) !important; | ||
| } |
There was a problem hiding this comment.
I would suggest minimize the use of !important unless its absolutely necessary for the override.
Also both popups are missing a border/box shadow where the arrow side of the popup.
There was a problem hiding this comment.
For this part of the code, I actually just copy and pasted it from the desktop version. I notice that the desktop version also have the same issue. So maybe for this PR, I include only these changes to fix white background, and perhaps when I had time I would look into the missing border issue and create separate PR for it. Let me know if you're ok with this
There was a problem hiding this comment.
Sure but it is already an issue on desktop version (misuse of important when not necessary), just because something has existed for a long time, it shouldn't be perpetuated if not absolutely necessary.
At some stage I intended to cleanup more of these misuses but it is a job for the ages, easier to re-write style, now Ive no time for these.
There was a problem hiding this comment.
Yeah, only the last rule would need important for the .mwe-popups-fade part, or conversely, use the specific selector that appears in the vendor style: .mwe-popups .mwe-popups-extract[dir="ltr"]::after
I think the second option is a better choice here, also those unrelated selectors for permissions probably shouldn't be carried over unless that element actually exists in the mobile style.
There was a problem hiding this comment.
So after testing, just the above vendor-provided selector is necessary for that last rule, and !important can be removed from all of them. Everything else is good.
| .mwe-popups-extract .mwe-popups-fade, | ||
| .mw-mmv-permission-box .mw-mmv-permission-text .mw-mmv-permission-text-fader { | ||
| background-image: linear-gradient(to bottom, transparent, var(--gray-2)) !important; | ||
| } |
There was a problem hiding this comment.
Yeah, only the last rule would need important for the .mwe-popups-fade part, or conversely, use the specific selector that appears in the vendor style: .mwe-popups .mwe-popups-extract[dir="ltr"]::after
I think the second option is a better choice here, also those unrelated selectors for permissions probably shouldn't be carried over unless that element actually exists in the mobile style.
| .mwe-popups-extract .mwe-popups-fade, | ||
| .mw-mmv-permission-box .mw-mmv-permission-text .mw-mmv-permission-text-fader { | ||
| background-image: linear-gradient(to bottom, transparent, var(--gray-2)) !important; | ||
| } |
There was a problem hiding this comment.
So after testing, just the above vendor-provided selector is necessary for that last rule, and !important can be removed from all of them. Everything else is good.
* Remove unnecessary uses of !important (Eddie James Carswell II) * Miscellaneous white background fixes (#162) (Jason Wihardja) * Update wikipedia-dark.user.css (TotalCaesar659) * Fix keyboard keys again (TotalCaesar659) * v3.4.27 (Rob Garrison) * Fix container background. Closes #159 (Rob Garrison)
This is to fix several more issues with white background. All examples are taken from the page https://en.wikipedia.org/wiki/Riemann_hypothesis
Commit 8304d00 fixes white background for tables on desktop
Commit 335aebb fixes white background on mobile quotebox
Commit 958f811 fixes white background on mobile lazy loading placeholder image
Commit e04f567 is to remove redundant style (has been mentioned on line 2870 of the code). In addition, background color for mobile is also adjusted so that texts on svg image displays correctly