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

Coy: Set z-index to make shadows visible in coloured table cells #3161

Merged
merged 2 commits into from Oct 24, 2021

Conversation

hoonweiting
Copy link
Contributor

@hoonweiting hoonweiting commented Oct 23, 2021

Just realised that the problem of Coy's shadows disappearing in coloured table cells still exists, at least on Prism's website.

This PR sets the z-index of pre to 1, and .toolbar to 2, so the shadows are visible. I'm not super sure if this is the way to go (mainly idk if this will break people's stuff on their own sites if they update their theme, but if they have a problem they can fix it on their end?), but I guess this change is along the same lines as the previewers popup from a while back so it's fine?

Current This PR
image image

@github-actions
Copy link

@github-actions github-actions bot commented Oct 23, 2021

No JS Changes

Generated by 🚫 dangerJS against 419f9ed

@RunDevelopment
Copy link
Member

@RunDevelopment RunDevelopment commented Oct 23, 2021

I hate Coy so much...

Thanks for the PR @hoonweiting!

Unfortunately, this bug has (almost) nothing to do with Toolbar. Coy is a strong and independent theme and doesn't need no plugin to be bugged. It seems like Coy's shadows just don't work when you set the background color of the parent element.
Frankly, I am surprised this bug wasn't found sooner. (Maybe people just didn't notice/mind? I've seen websites using Coy where code blocks didn't have shadows. I thought it was intentional. Maybe it wasn't?)

So that means that your PR won't really fix the problem.

My idea would be to remove z-index from :before and :after and instead add a z-index: 1 to the code element. This does solve the shadow issue but it also makes the toolbar disappear. The code element is then rendered above the toolbar. However, I would actually argue that this (the rendered below code issue) is a bug with Toolbar. Remember #3074? Same thing. We should fix that too with a z-index: 9 (or 10?) for div.code-toolbar > .toolbar.

What do you think @hoonweiting?

mainly idk if this will break people's stuff on their own sites if they update their theme

I wouldn't worry about that too much. By the nature of CSS, (almost) every fix is also a breaking change.

@hoonweiting
Copy link
Contributor Author

@hoonweiting hoonweiting commented Oct 24, 2021

@RunDevelopment Yeah, I like your idea much better! Should I send in a separate PR for the Toolbar fix?

Coy has its charms, and to be fair, I almost didn't notice the missing shadows the first time round either! And I just noticed something else on the front page of the website:

Tomorrow Night Coy
image image

I just filed an issue for it: #3162. I wouldn't have noticed this if I hadn't happened to have the Coy theme selected and decided to scroll down on the front page for no real reason.

Also, I can't believe there's a whole guide out there on semantic versioning! Interesting stuff, thanks!

@RunDevelopment
Copy link
Member

@RunDevelopment RunDevelopment commented Oct 24, 2021

Should I send in a separate PR for the Toolbar fix?

That would be great. Thanks!

I just filed an issue for it: #3162

Good find and it's not even Coy's fault.

@RunDevelopment RunDevelopment merged commit 79f250f into PrismJS:master Oct 24, 2021
12 checks passed
@RunDevelopment
Copy link
Member

@RunDevelopment RunDevelopment commented Oct 24, 2021

Nice. Thank you @hoonweiting!

@hoonweiting hoonweiting deleted the coy-z-index branch Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants