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

recent 1em addition breaks embedding in Reveal.js #3576

Open
garretwilson opened this issue Oct 12, 2022 · 5 comments
Open

recent 1em addition breaks embedding in Reveal.js #3576

garretwilson opened this issue Oct 12, 2022 · 5 comments

Comments

@garretwilson
Copy link

garretwilson commented Oct 12, 2022

I have a site using a an old version of Prism (I'm not sure which version, but it's at least a year or two old; it may have been around v1.17.1) embedded in a presentation using an old version (probably around v3.8.0) of Reveal.js. You can see an example of the old, working version at https://confound.io/present/intro (at least until I upload a workaround); hit the right arrow key until you get to a code snippet (e.g. the "Configuration Interface" slide).

Everything looked nice. I'm just now updating to the latest Prism 1.29.0. Some time between the last version and this version, Prism added the following CSS:

code[class*="language-"],
pre[class*="language-"] {
  …
  font-size: 1em;

This one property setting breaks the code snippets in the Reveal presentation. Suddenly the font size of the code is huge! Only a few lines are visible, scroll bars appear, and one must scroll just to see all the code.

It would seem that the size of the <pre><code> blocks were already set up correctly in Reveal, and were working fine with Prism. But this recent change in Prism override the "natural" size of the code blocks, assuming that 1em was a good size for their fonts.

I have only started investigating. I'll experiment with a newer version of Reveal, and I'll investigate what additional things were changed in Prism.

But at this point could someone tell me why this change was made? It was working fine before; what did the 1em addition intend to fix? Looking at the older Prism code, it doesn't appear that Prism changed the font size at all before. And intuitively, doesn't that seem to be the correct approach? What makes Prism suddenly think that 1em is a good size for my code blocks? Shouldn't Prism defer to the size already on the page as per how the developer designed it?

Please help me understand the context of this change. As it stands it seems like a breaking change and I don't immediately see its purpose.

@garretwilson
Copy link
Author

garretwilson commented Oct 12, 2022

This seems to have been introduced in 878ef29 to "normalize the font-size of pre and code". It was part of pull #1791, which indicates tried to fix a Coy "line drift issue" on Microsoft Edge.

Unfortunately, although the pull request shows a screen shot of a "fixed" version, it doesn't link to any ticket number, so I can't find a discussion of the problem or any screen shots of the "before the fix" version. I'll keep digging, but perhaps there might be some better approach than just forcing a font size across the board?

@garretwilson
Copy link
Author

Unfortunately I'm not immediately finding a workaround.

The following changes the size, but it doesn't use the correct size:

.reveal code[class*="language-"],
.reveal pre[class*="language-"] {
  font-size: initial;
}

That resets the font back to its original size. We need a way to basically reverse the effect altogether of Prism's setting the font size.

I may just have to force my own font-size em value, but that's a last resort. Ideally I would leave it to Reveal to set the best size, but Prism is forcefully overriding that.

@RunDevelopment
Copy link
Member

Some of our themes and plugins assume that the <pre> and <code> elements to have the same font size (see PrismJS/prism-themes#79).

Unfortunately, although the pull request shows a screen shot of a "fixed" version, it doesn't link to any ticket number

Uhhmmm, it links #1790.

IIRC, the problem was that IE and old Edge had something like code { font-size: 80% } in their agent styles. This broke the above assumption.


So yeah, any solution to your issue also has to ensure that pre > code elements have the same font size (in absolute units) as their pre.

Also, I don't really understand what is going wrong with reveal.js. Do you have an example, where I can see this for myself?

@garretwilson
Copy link
Author

garretwilson commented Oct 26, 2022

Also, I don't really understand what is going wrong with reveal.js. Do you have an example, where I can see this for myself?

Yes, it's the link I mentioned it in the description, but I have since updated it with the latest Prism and a workaround. You'll need to use your browser developer tools.

First, here is the workaround I'm using. I don't like it because I want the surrounding context to set the correct size automatically instead of forcing it at a global level.

/*
  Restore correct Prism code block size when embedded in reveal.js.
  See [Issue #3576](https://github.com/PrismJS/prism/issues/3576).
*/
.reveal code[class*="language-"],
.reveal pre[class*="language-"] {
  font-size: 0.75em;
}

To see the difference, do the following:

  1. Go to https://confound.io/present/intro .
  2. Use <space> or <right arrow> until you get a slide with a code block (e.g. the "Configuration Interface" slide).
  3. Bring up the browser developer tools.
  4. Select the <pre> or <code> being used with Prism.
  5. In the developer tool styles , find the font-size: 0.75em mentioned above, and uncheck it to disable it. You'll see that the font size is suddenly enormous.

In previous versions of Prism I could embed it in Reveal.js just fine and the size would be correct. It is specifically the new font-size: 1em; that is causing the trouble.

@RunDevelopment
Copy link
Member

RunDevelopment commented Oct 26, 2022

Interesting. Reveal.js uses .reveal pre { font-size: 0.55em } (Prism's styles override this because of their higher specificity), and your workaround approximates this as 0.75em*0.75em=0.5625em.

I suppose all Prism needs is pre > code { font-size: 1em } to ensure that the pre and code elements have the same (absolute) font size. We don't strictly need font-size: 1em for pres nor for non-code-block codes, I think. This will need some testing.


Note to maintainers: When we fix this, we need to write down or even write tests for this kind of stuff. People on our themes having certain properties, and we should enforce this for all themes.

This includes all Prism Themes themes as well, of course.

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

No branches or pull requests

2 participants