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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Feb 15, 2021
Merged

Miscellaneous white background fixes #162

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
644601c
Fix for Mobile Popups (#156)
jason-wihardja Jan 24, 2021
4dbca97
Apply white background only on image thumbnail (Re: #156)
jason-wihardja Jan 24, 2021
8304d00
Fix White Table Background (Desktop)
jason-wihardja Jan 27, 2021
335aebb
Fix White Quotebox (Mobile)
jason-wihardja Jan 27, 2021
958f811
Fix White Lazy Image Placeholder (Mobile)
jason-wihardja Jan 27, 2021
e04f567
Remove Redundant Style
jason-wihardja Jan 27, 2021
069ad54
Extract style to affected selector only (Re: #162)
jason-wihardja Jan 28, 2021
bda8001
Merge branch 'master' into miscellaneous-white-bg-fixes
jason-wihardja Feb 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion wikipedia-dark.user.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1276,9 +1276,11 @@ 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 {
background-color: var(--gray-2) !important;
}
.mwe-popups .mwe-popups-container .mwe-popups-discreet {
background-color: var(--white) !important;
}
Expand Down Expand Up @@ -4192,6 +4194,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-discreet {
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 •
edited
Loading

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 •
edited
Loading

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