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

fix vertical stack background not working in scroll view #3568

Merged
merged 9 commits into from Feb 5, 2024

Conversation

bouzidanas
Copy link
Contributor

@bouzidanas bouzidanas commented Feb 5, 2024

Setting data-background-color on the parent <section> of a vertical stack works in normal slide view but not in the new scroll view ( view: "scroll" ).

In the following example,

<div className="reveal">
    <div className="slides">
        <section data-background-color="red">
            <section> Vertical slide 1 </section>
            <section> Vertical slide 2 </section>
        </section>
        <section> Next horizontal slide </section>
    </div>
</div>

the first two slides which are in the same vertical stack should have the same red background. In the normal view they do, but in scroll view they do not.

Looking in scrollview.js, I could not find any code that aims to transfer the effect of the <div class="backgrounds"> element which is responsible for establishing the background for stacks and slides so I added logic that checks if the "backgrounds" element exists and then copies the backgrounds of the immediate children to the corresponding scroll view page elements (in place of copying the presentation viewport background).

This seems to have resolved this issue for me.

@hakimel hakimel merged commit 67b5ec1 into hakimel:master Feb 5, 2024
hakimel added a commit that referenced this pull request Feb 5, 2024
@hakimel
Copy link
Owner

hakimel commented Feb 5, 2024

Thanks! This has been merged with a few minor naming tweaks.

@bouzidanas
Copy link
Contributor Author

bouzidanas commented Feb 5, 2024

Hi @hakimel,

Your changes makes sense and target the particular issue more precisely with the new isVertical argument. However, I now have a theme background copy issue on singular slides! So the copying of the theme background happens for the stacks but not for a stack of 1? I see that a background element is created for the single slide but its background style is empty as far as I can tell.

It could be something else going on, but when I revert back to my fork, the issue is gone. I will look into it more a bit now and later when I am able.

@bouzidanas
Copy link
Contributor Author

bouzidanas commented Feb 5, 2024

Ok, I think I figured it out. By copying the presentation background and not the slide-background div background corresponding to the single horizontal slide, we omit the background styles added by the CSS rule with the selector .reveal .slide-background which should apply to every slide I think.

So right now, I am leaning on this being a persisting bug. Meaning that it is an issue that still needs fixing.

I thought the problem was specific to my case where I am lazy loading my theme, but then I didnt understand why the code in my fork worked. If so, this might be causing other issues...

Let me know what you think @hakimel . I still think my code needs adjustment in the direction you provided to be more intentional and clear about targeting the case where a slide is part of a vertical stack and the case where the slide is not.

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

Successfully merging this pull request may close these issues.

None yet

2 participants