-
Notifications
You must be signed in to change notification settings - Fork 188
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
Allow slide background color to be set using a role #16
Comments
I'm also open to the idea of pushing this change upstream, but first I think we need to determine what we want the behavior to be. |
Could you give an example of what solution #2 would look like?
|
Solution 2 would walk through each section element and read the background color from CSS using getComputedStyle. If the background-color is not transparent (i.e., var slides = Array.prototype.slice.call(document.querySelectorAll('.slides section'));
slides.forEach(function(slide) {
var bgColor = getComputedStyle(slide).getPropertyValue('background-color');
// FIXME this check for transparency is fragile...we need to be more thorough!
if (bgColor != 'rgba(0, 0, 0, 0)') {
slide.style['background-color'] = 'rgba(0, 0, 0, 0)';
slide.setAttribute('data-background-color', bgColor);
}
})
Reveal.sync() I tried it and it works assuming I set the background color on the slide using CSS: <section id="_slide_three" style="background-color: red">
<h2>Slide Three</h2>
<div class="paragraph"><p>Is very red.</p>
</div>
</section> Note that we may be able to hook into loading (or invoke before reveal.js is loaded) to avoid the call to |
In fact, if we add the following snippet of code just above where we initialize reveal, it works! var slides = Array.prototype.slice.call(document.querySelectorAll('.slides section'));
slides.forEach(function(slide) {
var bgColor = getComputedStyle(slide).getPropertyValue('background-color');
// FIXME this check for transparency is fragile...we need to be more thorough!
if (bgColor != 'rgba(0, 0, 0, 0)') {
slide.style['background-color'] = 'rgba(0, 0, 0, 0)';
slide.setAttribute('data-background-color', bgColor);
}
}) I'd just like to make the check for transparency a bit more robust, then we can move forward with this. Now, it's possible to use the stylesheet (and thus theme) to set background colors for slides. For instance, you can set the background color for topic slides and just assign the |
wdyt @obilodeau? Should we move forward with this change? |
We should probably set data-background-color only if it is not already set. That way, the CSS has a lower priority than a hard-coded value for backwards compatibility and such. |
I really like where your idea is going. I do attributes shorthands like these all the time:
and then alter text color with a theme with this:
I think doing all in one place would be much better. CSS is the best place since we would be separating content from presentation. I'll re-evaluate what upstream has to offer now regarding pure CSS slide customization solutions and I'll update with my findings. |
Indeed, using CSS is ideal. But the way Reveal.js is implemented, the CSS requires a little bit of help to be applied to the canvas. That's because there's no fixed canvas that you can draw on with CSS. What we're trying to do here is bridge this divide. |
I think we might be able to do something interesting with CSS conditionals on the data-state attribute. A |
The That said, if you look at this example (from https://github.com/asciidoctor/asciidoctor-reveal.js/edit/master/test/slide-state.adoc): = Title
:topic: state=title,background-color=white
:customcss: slide-state.css
== First slide
Content
[{topic}]
== Topic slide
== Second slide
Content We realize that we still need to assign presentation-specific instructions in both AsciiDoc and CSS which we don't like. So we should try to fix this using your proposed solution 2. |
Have we made a decision about what direction to take here? How can I help move the conversation forward? |
To me, the ultimate Zen of slide writing would be: = Title
:customcss: company_template.css
== First slide
Content
[state=topic]
== Topic slide
== Second slide
Content and then the CSS would have the color assignment like this: @import 'https://fonts.googleapis.com/css?family=Baloo+Bhai';
/* font example */
section[data-state="topic"] h2 {
font-family: 'Baloo Bhai', cursive;
font-size: 4em;
}
/* color example */
section[data-state="topic"] {
background-color: red
} Is this something that solution 2 can do? For extra syntactic sugar, roles could be automagically mapped to the = Title
:customcss: company_template.css
== First slide
Content
[.topic]
== Topic slide
== Second slide
Content But then you need to remember this magic when writing your CSS. So, if you like it, all we are waiting for is a PR. If your PoC works, send it in, I'll do the docs and testing. |
PR sent! The next step would be to allow the background image to be set via CSS (using a role). But that should be a separate issue that can build on the code from this one. |
@obilodeau regardless of how you add an attribute to the slide (whether it's a data attribute or a CSS class), the point remains that Reveal.js does not scope the CSS properly. The CSS applies to the content part of the slide, not the whole viewport. Therefore, it's not possible to paint the viewport using CSS. But that's what most users expect to happen (drawing from discussions I've read). With this patch, we promote the CSS from the slide to the viewport by piggy backing on the behavior controlled by the data-background-color attribute. |
…-css resolves #16 allow background color of slide to be set using CSS
The slide background color can only be set by hard-coding the value in the
data-background-color
attribute. This requirement has the following drawbacks:Better would be to allow the background color to be set using a role. The role introduces a layer of indirection between the content and the presentation and it can be used to change the color across all corresponding slides.
Unfortunately, reveal.js doesn't support setting the background color of a slide via CSS. The reason is because reveal.js reads the background color directly from the
data-background-color
attribute on the section and applies it to a floating canvas layer. If you attempt to set the background color of a slide using CSS, you'll notice that it only colors the content portion of the slide.Since we have complete control over the generated HTML, we can patch in a solution that allows the background color to be controlled using a role. I've come up with two possible solutions.
Solution 1: Read the color value from an attribute
If we agree on preset section role names, we could map the role to an AsciiDoc attribute and assign that value to the
data-background-color
attribute on the section when the HTML is generated. Here's an example:The resulting section element for the "Topic Slide" would be something like:
We could follow a fixed pattern so that we don't have to decide on preset class names (something like
section-*
).Solution 2: Use JavaScript to assign the
data-background-color
dynamicallyWe could inject JavaScript that runs after the presentation loads to read the effective background of each slide (from calculated styles) and apply those background-color values to the
data-background-color
attribute while removing the background-color from the section. This is the cleanest approach, but will require some research.I'm happy with Solution 1 until we can get something more formal in place. Perhaps we can implement as two steps since Solution 1 honors the same contract in the AsciiDoc content as Solution 2...it just requires a shim (the document-level color attribute).
The text was updated successfully, but these errors were encountered: