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 theme shadow doesn't correctly wrap around floats. #2164

Closed
garretwilson opened this issue Jan 7, 2020 · 12 comments
Closed

Coy theme shadow doesn't correctly wrap around floats. #2164

garretwilson opened this issue Jan 7, 2020 · 12 comments

Comments

@garretwilson
Copy link

garretwilson commented Jan 7, 2020

Years ago I had turned off the Coy theme shadow as we discussed in #894, but I had forgotten exactly why. (I had thought it only had to do with scrollbars in generated PDFs in #859.) So when I updated to the latest Prism a few days ago and published my entire software development course, I left the Coy theme shadow on.

Now I see that the Coy theme shadow is broken when wrapping around flows. Here are a couple of examples:

@garretwilson
Copy link
Author

garretwilson commented Jan 7, 2020

Note that this may or may not be related to #965.

@RunDevelopment
Copy link
Member

What should the correct behavior be? The reason why we see the white background is that the pre has the dimensions of the parent figure which spans across the whole width of the page (sans some margin). The overlap with other blocks comes from the usage of float which usually doesn't far well with display: block from my experience.

So we could either want the block to have the same width as its parent, in which case the solution is quite simple. (clear: both; for the pre):

image

Or we might want the code block to behave like text, so it "breaks" early.

image

I'd go with the former solution. If you still want the code block to "break", then you can still change the parent figure. (display: grid; works for some reason.)


this may or may not be related to #965.

It doesn't seem to be related.

@garretwilson
Copy link
Author

What should the correct behavior be?

That's the easy part. The thing is supposed to appear as a shadow of the box containing the code. Therefore the shadow should begin at the blue vertical line and extend only to the end of each horizontal blue line in the Coy theme. If the shadow extends to the left of the vertical blue line, or to the right of the horizontal blue lines, there is a bug. The fact that it may be "doing what we told it to do" is beside the point. The point is that it isn't correctly functioning as a shadow, and thus displaying buggy behavior.

The reason why we see the white background is that the pre has the dimensions of the parent figure which spans across the whole width of the page (sans some margin).

OK, so if you follow that logic, what you're really saying is that there is a bug in Prism.js that is not making the Coy theme code section extend to the full length.

In fact now I seem to remember that in fact these Prism.js code blocks did in fact extend the full length and actually overlapped the float elements. (I had forgotten this.) So if I'm not mistaken, there has been some changes that prevent the code blocks from overlapping the floats.

I suppose this is an interesting case because normally text in a block would flow around the float, but the whole point of <pre> is that the text is preformatted and does not wrap, so it's almost as if two contrasting forces are at play.

I haven't completely decided, but I sort of like the way that <pre> is now behaving sort of like text. The problem is the shadow. It might be interesting to leave the <pre> part breaking as it does now, and just fixing the shadow.

But anyway, if you decide to put it back the way (I seem to remember that) it used to be in previous versions, where the entire code block overlaps the floats; or if you decide to allow it to break the way it is doing now; whichever way you decide, the shadow has to match. It has to match without any structural changes in the code. Either the shadow matches the thing it's being a shadow to, or it isn't being a very good shadow and it's broken. That's the straightforward part, and to me that seems very easy to understand and not ambiguous at all.

@RunDevelopment
Copy link
Member

It seems like Coy is the only theme that exhibits this behavior. All other themes work as intended in that the pre doesn't overlap with other elements.

image

The cause is that the overflow of the pre is set to visible. Any other overflow value will fix the bug and will make Coy behave like all other themes but it also breaks Coy's design completely.

So I tried to rewrite the Coy theme to not use overflow: visible on the pre but couldn't do it. My problem was is overflow: visible is the only overflow value that allows you to draw outside the pre. Without it, the shadows aren't possible, at least for me.

I'll make a PR with my first fix (clear: both). By far not perfect but I hope it'll solve your problem.

@garretwilson
Copy link
Author

I'll make a PR with my first fix (clear: both). By far not perfect but I hope it'll solve your problem.

I believe that in some cases I actually float the <pre> itself, or at least the <figure> containing the <pre>. That is, I would have the code floated to e.g. the left while e.g. an <aside> would be floated to the right.

So please don't add some workaround that would break floating the code itself. (You don't clear floats on the other themes, do you?)

It may be that you simply have to mark this as a known bug. I can go back to disabling the shadows, as I did for years. I was just hoping the issues had been fixed by now, and I was filing a bug now that I have publicly published pages that illustrate the problem. But I can live without the shadows, and I can turn them off again. There are much bigger issues to tackle. It's a shame, but I'd rather not have heavy-handed workarounds if they reduce the flexibility of usage.

@mAAdhaTTah
Copy link
Member

Yeah, coy & funky are both complicated but pretty themes. I appreciate your attitude on this because that was my gut reaction as well. If you need, you can include the workaround in your project CSS, if you want the maintain the shadows. Otherwise, I don't know if it's really "fixable" because it seems kinda fundamental to the implementation, but I'd have to dig into it.

Going to close this out, as I don't think this is actionable, but feel free to comment and we can reopen if you believe otherwise.

@garretwilson
Copy link
Author

garretwilson commented Jan 9, 2020

If you need, you can include the workaround in your project CSS, if you want the maintain the shadows.

I really think you need to document the differences among the themes, the bugs each have, and the workarounds, though. It's not good to have people happily download what according to the site is a wonderful, beautiful, perfect thing; and then halfway through a project find out it breaks their layout because they weren't informed.

@RunDevelopment
Copy link
Member

I'll add it to the known failures page.

(The page even showcases this bug rn...)

@garretwilson
Copy link
Author

garretwilson commented Jan 9, 2020

(The page even showcases this bug rn...)

Haha. I just noticed that. Was that already there? Oh, the irony.

Anyway, thanks. In the least maybe this will keep me from forgetting several years from now why I removed the shadow and (as I did now) trying to put it back and then wasting lots of time realizing I have to take it out again (across all my sites—ack!).

@garretwilson
Copy link
Author

(The page even showcases this bug rn...)

Well if it shows up so often, it might be worth thinking about it some more over the coming weeks.

@garretwilson
Copy link
Author

I just noticed that using reveal.js I get (very light) shadows for my Prism Coy code blocks for free, even when I turn them off in Prism (see #965). Here is the code they use; I don't know if it would fix some of the Prism Code bugs or not.

.reveal pre {
  display: block;
  position: relative;
  width: 90%;
  margin: var(--r-block-margin) auto;
  text-align: left;
  font-size: 0.55em;
  font-family: var(--r-code-font);
  line-height: 1.2em;
  word-wrap: break-word;
  box-shadow: 0px 5px 15px rgba(0, 0, 0, 0.15);
}

Unfortunately I don't have time to dig into this. I'm just turning off the Coy drop shadows. (See also comments in #2170.)

@RunDevelopment
Copy link
Member

I don't know if it would fix some of the Prism Code bugs or not.

It likely would. We already have a "Coy without shadows" theme, maybe we could also add something like an "Alternative Coy" with those shadows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants