Play icon for homepage featured video post thumbnail #923
Conversation
…the CSS2.1 and CSS3 specs do not define how they operate with <img> And therefore, browsers don't display ::before or ::after on an <img>. So much for this approach, which would put the play icon as an svg behind the image.
…things using .is-video containing an img.
|
Using this grep command, I found no child theme PHP, CSS or LESS files using |
|
@benlk I'm actually not sure we need/want this for the top story in this homepage layout. There are a number of issues that could come up with the except/headline box covering the icon and making it look jumbled and things like that so It's probably better to just use the play icon for smaller images. Ryan built a solution for this for another site that you might want to reference (just so we're consistent in our implementation), see: https://bitbucket.org/projectlargo/theme-gijn-impact/src/5afe5a76b2153e510f193804ce5047b510544e8c/less/style.less?at=master&fileviewer=file-view-default#style.less-191 |
|
That is a good reason not to use it in the homepage top. I put it there because it was large and a good example of what this would look like, but it can be removed. Here's a list of other places we use thumbnails:
Is there any place other than the homepage top where we would not want the play icon to show up? The reason I'm not in favor of Ryan's solution is that it adds extra elements to the page, which have positioning requirements and font requirements. This PR makes the image transparent, adds a small inline SVG to the CSS, and uses the SVG as a background on one of the image's parent elements. This doesn't have positioning requirements that could be messed up by child theme CSS. I'm not sure how easy it would be able to change the appearance of the play icon here in a child theme. It would involve writing a different SVG and inlining it, instead of changing `.play-button-overlay .icon-play::before { content: '\60'; } in Ryan's version. That is a point in favor of Ryan's version. |
|
Mostly, I want to do this in a way that is straight CSS, so we're not adding elements to the page that child themes will have to remove or hide if they don't want the icons. |
|
all of those other places seem fine except maybe the prev/next links, I'd need to see that in context but I think the arrows might actually be confusing in that context (confusion re: prev/next vs. play, that is). On the CSS thing, I'll defer to @rnagle for a judgment call on that |
{
background: url(fallback.png);
background-image: url(image.svg), none;
}
|
|
I'd like to not spend much more time on this at this point. If a lot more testing is necessary, let's go with the non-css version that we know works and then come back to this as an enhancement at a later date. |
|
This doesn't play well with the LESS customizer: #936 The assumption that an On category archives, it works for the primary post, which is alone in a But the I stopped checking after that. I think this is better than adding the icon-play elements as we did in gijn-impacts's partials, but at this point we're going to be modifying all the partials anyways. CSS is a smaller set of modifications to the partials than the |
|
@benlk Instead of embedding the SVG inline, just save an SVG file and load it via If you need to add markup to specific templates or template partials, then do that. For theme's that override a template or partial where markup changes, the icon simply won't show. This seems acceptable in the short-term -- means we can finish this without a lot more testing. |
…fix floats, generally set fire to this PR and no more thumbnails ever
|
Checked all the places that we use the partials, and where thumbnails are used. Changes noted below.
Did you know that partials/content-tiny.php is only used at all in one widget, the Series Posts widget? |
|
Yeah, I noticed that a while ago. Not sure we really need it, to be honest. |
…css files by running grunt less and cssmin
…yles Play icon for homepage featured video post thumbnail
Changes
The full-width image template gains as a class the output of
largo_hero_class(), which is.is-videofor posts with the featured video embed option.The
.is-videoclass gains a black background and a white triangle background for the play button.The
imginside.is-videobecomes 50% transparent, to show the white background.This also incorporates #922 for #921, forcing the
.home-topdiv to 100% the width of its container. This is only used on the Single template and its derivatives.Why
For #836 and WE-15
Questions:
.is-videofor something else?.home-topfor something else?Possible effects on child themes:
Are they using the
.home-topclass for something else? This is a very-low-specificity selector, so it will probably be overriden by a child theme's more-specific selectors, and in any case the child theme's CSS is loaded after Largo, so it would override.home-top'swidthif the child theme set thewidth.Any lower-specificity-score selector is going to be using an element name.
In the event that there's a competing class applied to the same element by a child theme, I think the child theme's class's width would take priority, though I'm not sure about the relative scoring of a same-weight selector applied to the same element from a later, overriding stylesheet.