search

hakimel / reveal.js

The HTML Presentation Framework
https://revealjs.com
MIT License
67.78k stars 16.64k forks source link

regression: controls disappear; don't account for header #2470

Closed garretwilson closed 2 years ago

garretwilson commented 5 years ago

I have an HTML document with a header, with a menu in the header:

<body>
  <header>...</header>
  <main class="reveal">
    <div class="slides">
    ...
    </div>
  </main>
</body>

On an old version of reveal.js, this worked just fine, and the controls appeared. You can see it here:

https://confound.io/present/intro

I'm developing a new site with an almost identical layout. I downloaded the latest reveal.js v3.8.0. Unfortunately, unless I'm mistaken, it seems to be calculating the size of the presentation without taking into consideration the height of the menu in the header. The controls disappear below the screen!

As you can see from the link above, this was working fine in earlier versions. I have confirmed that reverting to a reveal.js from 2017 fixes the problem. I'll have to revert to the 2017 version, but unfortunately that means that I'll have to add back my linear navigation workaround for #1904.

garretwilson commented 5 years ago

Is there any possibility this can be addressed relatively quickly?

I will be presenting at ApacheCon Las Vegas in two weeks, and I'll be using reveal.js as my main presentation platform. If this can't be fixed soon I'll have to revert to an older version of reveal.js. 😞

garretwilson commented 5 years ago

Uh oh, this is bad. If I revert to the 2017 version of reveal.js, I see the controls again, but it breaks Font Awesome. It looks like reveal.css is adding a font: inherit to almost everything, which removes the font-family: Font Awesome 5 Pro of the icon Font Awesome added.

It looks like I'll have to publish my presentation without visible controls, unless someone knows a workaround.

garretwilson commented 5 years ago

Here is my presentation I intend to show at ApacheCon next week. You can see that it makes the navigation controls disappear:

https://guise.io/mummy/present/clean-urls

Again, this is a regression from previous versions.

A fix or a workaround would be greatly appreciated.

garretwilson commented 5 years ago

This is disappointing. I'm two days away from presenting at the official Apache Software Foundation conference, and it looks like I'll have to show and publish a presentation with no visible controls. 😞

hakimel commented 5 years ago

Your page ends up being too tall since the "reveal" element has its height set to 100% and your header/footer elements are added on top of that.

You should be able to fix this by making the containing element (body) a flex column.

body {
    display: flex;
    flex-direction: column;
}
garretwilson commented 5 years ago

You should be able to fix this by making the containing element (body) a flex column.

But I don't want to change my site. I don't want to change my CSS framework.

Did you see the part of my description that explains this is a regression? If I drop in a version of reveal.js from 2017, the problem does not occur. If it worked then, it can work now, right?

garretwilson commented 2 years ago

Downloading the latest v4.3.1 I see this is still an issue.

The code for the main reveal class looks like this:

.reveal {
  position: relative;
  width: 100%;
  height: 100%;
  overflow: hidden;
  touch-action: pinch-zoom;
}

I guess the point of the position: relative is to create a new stacking context, because the controls use absolute positioning to place themselves at the bottom, right-hand corner. But if you take away the position: relative, the controls will just go to the bottom, right-hand corner of the stacking context another level up. And since the width and height are being calculated based upon the parent context anyway, wouldn't that be more appropriate?

As I mentioned this didn't use to be a problem. You might look at the old code to see whether it used position: relative.

In any case for my purposes I can fix this problem just by changing position back to static. The controls will still be placed at the bottom, right-hand corner of whatever I'm placing the slide show in. It looks good to me, and works around the problem:

/*
  Prevent slide controls from disappearing if there is e.g. a menu above.
  See [Issue #2470](https://github.com/hakimel/reveal.js/issues/2470).
*/
.reveal {
  position: static;
}
garretwilson commented 2 years ago

Oops I take that back. Changing from relative to static positioning will cause the presentation to cover up the menu with its absolute positioning (even though you can't see it), so it's no longer possible to click the menu. I'll look more closely at this.

garretwilson commented 2 years ago

Did you see the part of my description that explains this is a regression? If I drop in a version of reveal.js from 2017, the problem does not occur. If it worked then, it can work now, right?

Ah, so I'm comparing the latest version with a non-broken version from 2017. It looks like thew newer reveal.js decides it wants to add a reveal-viewport style to my entire page <body>! The old version didn't do that. So going back to the example I gave at the beginning of this ticket:

<body>
  <header>...</header>
  <main class="reveal">
    <div class="slides">
    ...
    </div>
  </main>
</body>

The older version would simply cover up the entire <main>. But the newer version puts a reveal-viewport on the <body> with relative positioning. Somehow the new version is making calculations off the <body>, which isn't good. It's OK for reveal.js to expand elements as needed, but it shouldn't be making calculations based upon elements outside the element marked with reveal.

Martinomagnifico commented 2 years ago

Hi Garret,

I've also struggled a bit with the layout. I think that the -viewport class is needed for situations where a presentation is embedded in a page. As for displaying a header (not a "head", which is something different), look at the demo.css in https://github.com/Martinomagnifico/reveal.js-simplemenu and the placement of that header in the markup in index.html (inside the .reveal div, a sibling to .slides). Because of that nesting, elements in the header can react to changes in .reveal, like "has-dark-background".

garretwilson commented 2 years ago

As for displaying a header (not a "head", which is something different) …

Thanks for the catch. That was a typo. (I've now edited and fixed it in the comments.) I'm using <header><nav> … with my own stylesheet for a horizontal menu in the header.

… placement of that header in the markup in index.html (inside the .reveal div, a sibling to .slides).

No, I'm not going to rearrange my page structure to work around a reveal.js problem. reveal.js needs to play nicely and stay within whatever element I put it in.

Martinomagnifico commented 2 years ago

One of the cool things with Reveal is that a page can have several embedded presentations. Each of those will have a .reveal element. Placing a header outside the .reveal element will then only show 1. That’s one of the reasons that elements like controls, progress are placed relative (but absolute) to each of those presentations. A header is exactly a similar element.

garretwilson commented 2 years ago

I've improved upon my workaround above. What was covering up my menu as the backgrounds <div> reveal.js creates. I take away its relative positioning, and I can access the menus again. So with the following workaround the controls don't halfway disappear below the page, and the presentation doesn't cover up the menu header.

/*
    Prevent slide controls from disappearing if there is e.g. a menu above.
    See [Issue #2470](https://github.com/hakimel/reveal.js/issues/2470).
*/
.reveal {
  position: static;
}

.reveal .backgrounds {
  position: static;
}

There are likely other implications of this I haven't found yet, so I'll keep investigating.

garretwilson commented 2 years ago

Placing a header outside the .reveal element will then only show 1.

Will only show one what? I don't understand what you're saying here. Will only show one header? Will only show one presentation?

Martinomagnifico commented 2 years ago

Only one header. Reveal is the main element and class for each presentation. Logically placing a header in each presentation needs an element to put it in. And because there is only 1 body, something else is needed. One or multiple .reveal elements.

garretwilson commented 2 years ago

Only one header

Of course. If I have my own header outside the reveal element, then it should only be one header. Everything I place outside the reveal element should be exactly as I have it, and reveal.js shouldn't touch it. That's my entire point.

But instead reveal.js is reaching its tentacles out and expanding its influence to my entire <body>, asserting control over all the space even that my <header> takes up. I don't want it to do that. I want it to stay inside the reveal element and leave the rest of the page alone. (It can expand the element it's in—that's fine. That's a different thing altogether. Even better is let me style the element to make it as big as I need it to be.)

garretwilson commented 2 years ago

Reveal is the main element and class for each presentation.

Right. It shouldn't be reaching out and mucking around with the <body> as it's doing now.

Logically placing a header in each presentation needs an element to put it in.

But I'm not putting the header in the presentation! The header is outside the presentation! It has nothing to do with the presentation.

Here, look at this reveal.js version from around 2017, when it wasn't broken: https://urf.io/present/why (Note that I'll update this once I've found a workaround, so this isn't guaranteed to stay the way it is now.) This was before it added that reveal-viewport thing. You'll see that the presentation stays within the reveal element. Using the developer tools, you'll see that the presentation doesn't extend to the entire <body> (as the current version does).

garretwilson commented 2 years ago

This was before it added that reveal-viewport thing.

While it's true that this is a new addition, I'm still not sure that is the root of the problem of the presentation controls halfway disappearing.

But instead reveal.js is reaching its tentacles out and expanding its influence to my entire <body>, asserting control over all the space even that my <header> takes up.

While it's true that reveal.js is now mucking around with other elements, which I don't like, it may have been my half-baked workaround that was making it cover the menu.

I'll keep investigating to try to find a more pinpointed workaround.

Martinomagnifico commented 2 years ago

Hi Garret, that's indeed what's happening in the newer versions. But is the header not part of the presentation? I understand that it is not part of the slides.

garretwilson commented 2 years ago

But is the header not part of the presentation?

No, in no way, shape, or form. It is part of the page structure. I show this plainly in the description of this ticket.

hakimel commented 2 years ago

Try setting the embedded config option to true. That will make reveal.js use the the .reveal element as its viewport.

More info: https://revealjs.com/presentation-size/#embedded

garretwilson commented 2 years ago

Try setting the embedded config option to true. That will make reveal.js use the the .reveal element as its viewport.

~Maybe I'm doing something wrong here?~ Ack, sorry! One second. I copied from the docs, which used false. 😅

hakimel commented 2 years ago

You'll want to set it to true. Embedded defaults to false so that the presentation covers the full page.

garretwilson commented 2 years ago

You'll want to set it to true.

Yep; see my updated comment above. hehe But I should notes the docs are a bit confusing. They say, "By default, reveal.js will assume that it should cover the full browser viewport. If you want to embed your presentation within a smaller portion …" and then go on to show an example with embedded set to false (which is the default). From the wording, it sounds like they are going to show an example of embedding the slides, so you expect the example to show true. That's probably a typo, but that's why I just copied and pasted without looking at the exact value, because I had read the text above it.

garretwilson commented 2 years ago

Try setting the embedded config option to true. That will make reveal.js use the the .reveal element as its viewport.

Oh wow!!

So immediately my slides disappeared, but that's not bad, because I realized that finally it's actually behaving like I would want it to behave in a perfect world—that is, I have complete control over how big it is, and it doesn't mess with the rest of my page!!

@hakimel, I don't know what to say. Let me test it a little more to make sure it's working, but this may be a dream come true. It seems too good to be true at the moment.

garretwilson commented 2 years ago

@hakimel, I have some good news and bad news. I turned on embedded, and then as a test I added height: 100% to both <html> and <body>. My presentation appeared again, filling the screen.

The good news is that the reveal-viewport class is now on the same element as the reveal element (not on <body>), so that's a step forward.

The bad news is that the navigation controls are still stuck partially below the window as before. So there is some bug introduced since 2017 that calculates the size incorrectly. Somehow it is calculating the entire size of its parent (which incorrectly would include its sibling <header> element, not just the size of the reveal element.

garretwilson commented 2 years ago

I think maybe I can explain why the navigation controls are below the screen. It's probably what you were telling me above a couple of years ago. You noted that the presentation is set at 100% height, and after researching to refreshing my memory, 100% means the height of the parent, which is the height including the header. So the same height of the header is the same amount it falls below the page.

So I understand and agree with what you said. What I don't yet understand is why it didn't happen in 2017 (as you can still see from the live link using the 2017-ish reveal.js).

garretwilson commented 2 years ago

@hakimel, I think I found the difference!! 🎉

I think that with the version from 2017, the exact same thing was happening as far as the height. That is, for the document structure I described here, the presentation was getting the same height as the <body>, which means because of the header technically it was going below the screen. But (as with the current version when the presentation is not embedded), there is actually part of presentation cut off below the screen!!

So why did the 2017 still show the navigation controls without them getting cut off? The 2017 reveal.js version used position: fixed instead of position: absolute for the <aside class="controls">!! In other words, they were placed relative to the bottom, right-hand side of the screen instead of relative to the bottom, right-hand side of the presentation, as they are now. So even though (unbeknownst to me) the presentation was cut off a little at the bottom, the controls still showed because then they were specified relative to the screen.

Now that I understand the difference, I think that the way it is now (relative to the presentation) is actually the more correct approach. And since you've pointed out the embedded option, that gives me more control over the size of the presentation. So I think all I have to do now 🤞 is to use the best layout mechanism to make the presentation fit into the remaining space. I'll report back.

manzo2202 commented 2 years ago

ในวันที่ ศ. 14 ต.ค. 2022 01:48 น. Garret Wilson @.***> เขียนว่า:

@hakimel https://github.com/hakimel, I think I found the difference!! 🎉

I think that with the version from 2017, the exact same thing was happening as far as the height. That is, for the document structure I described here, the presentation was getting the same height as the , which means because of the header technically it was going below the screen. But (as with the current version when the presentation is not embedded), there is actually part of presentation cut off below the screen!!

So why did the 2017 still show the navigation controls without them getting cut off? The 2017 reveal.js version used position: fixed instead of position: absolute for the

garretwilson commented 2 years ago

OK this is resolved and everything is looking good. Thank you @hakimel and @Martinomagnifico for helping me investigate this.

This was not a bug after all. Let me give a summary:

Here is the general approach I used for getting the presentation to fill the middle of the page under the menu/heading (hard-coded styles for example only):

<html>
…
<body style="display: flex; flex-direction: column">
  <header>...</header>
  <main class="reveal" style="flex: 1">
    <div class="slides">
    ...
    </div>
  </main>

  <script>Reveal.initialize({embedded: true});</script>
</body>
</html>

If you're using my Guise Skeleton "bare-bones" CSS framework (still in development, but nearing v0.1.0 release), you can do it like this:

<html class="guise-skeleton">
…
<body class="page layout-flex along-block">
  <header class="lay-fitted">...</header>
  <main>
    <div class="slides">
    ...
    </div>
  </main>

  <script>Reveal.initialize({embedded: true});</script>
</body>
</html>

The link in the description has now been updated with this fix. It looks great!

https://confound.io/present/intro

Thanks again for your patience and explanations—and for a great library.