Skip to content
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

Miscellaneous white background fixes #162

27 changes: 25 additions & 2 deletions wikipedia-dark.user.css
Original file line number Diff line number Diff line change
Expand Up @@ -1276,10 +1276,12 @@ regexp("https?:\/\/wiki\.(archlinux|mozilla)\.(org|jp)\/.*$") {
.messagebox, .errorbox, .warningbox, .successbox,
.suggestions-special .special-label, .reference-text,
.mwe-popups .mwe-popups-container, #viewsourcetext {
background-color: var(--gray-2) !important;
color: var(--gray-c);
}
.mwe-popups .mwe-popups-container .mwe-popups-discreet {
.mwe-popups .mwe-popups-container {
background-color: var(--gray-2) !important;
}
.mwe-popups .mwe-popups-container .mwe-popups-thumbnail {
background-color: var(--white) !important;
}
span[style="color:black"], span[style*="color: black"],
Expand Down Expand Up @@ -2851,6 +2853,7 @@ regexp("https?:\/\/wiki\.(archlinux|mozilla)\.(org|jp)\/.*$") {
.mw-echo-ui-notificationBadgeButtonPopupWidget-popup > .oo-ui-popupWidget-popup > .oo-ui-popupWidget-head,
.mw-echo-ui-notificationItemWidget:last-child {
border-color: var(--gray-5);
background-color: var(--gray-2);
jason-wihardja marked this conversation as resolved.
Show resolved Hide resolved
}
body[class*="skin-"] .mw-parser-output table.ambox {
border: 1px solid var(--gray-5);
Expand Down Expand Up @@ -4192,6 +4195,26 @@ regexp("https?:\/\/([\w\-]{2,}\.m\.(wiki(pedia|books|news|quote|source|versity|v
tr[bgcolor], td[bgcolor] {
background-color: var(--gray-28);
}
/* popups fixes */
.mwe-popups .mwe-popups-container {
background-color: var(--gray-2) !important;
}
.mwe-popups .mwe-popups-container .mwe-popups-thumbnail {
background-color: var(--white) !important;
}
.mwe-popups .mwe-popups-container a {
color: var(--gray-c) !important;
}
.mwe-popups-extract::after {
background-image: linear-gradient(to right, transparent, var(--gray-2) 50%) !important;
}
.mw-mmv-ttf-ellipsis::before {
background-image: linear-gradient(to right, transparent, var(--gray-2)) !important;
}
.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;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@jason-wihardja jason-wihardja Jan 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@the-j0k3r the-j0k3r Feb 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

/* these general selectorss have to go when adding not remove all */
td[style*="background"] {
background-color: var(--gray-28) !important;
Expand Down