OpenNews / opennews-source

Code refactor for Source, a website that spotlights work from the developers, designers, and data analysts at the intersection of journalism and tech: https://source.opennews.org
MIT License
5 stars 3 forks source link

Caption display weirdness #98

Closed kissane closed 7 years ago

kissane commented 7 years ago

It looks like our new caption style isn't getting applied to a bunch of our image captions, which I suspect is a CSS hook problem.

EXAMPLE URL: https://opennews-source-staging.herokuapp.com/articles/introducing-streamtools/

LOOKS LIKE THIS:

screen shot 2017-02-07 at 12 04 26 pm

MARKUP LOOKS LIKE THIS:

<div class="image-full-width-wrapper">
<img src="https://media.opennews.org/img/uploads/article_images/streamtools_nyt.png" alt="a more complex example with six graphical block-steps">
<p class="caption">Making a timeseries of <span class="caps">NYT</span>&nbsp;traffic</p>
                            </div>

SHOULD LOOK MORE LIKE THIS:

screen shot 2017-02-07 at 12 07 10 pm

WHICH IS ON A COVER IMAGE WITH THIS MARKUP:

 <div class="figure figure-article-lead">
                <figure>
                    <img src="https://media.opennews.org/cache/e5/20/e520bd53be742fe878d61eead28d5eb2.jpg" alt="" />

                    <figcaption>
                        <p>Also big and messy. (<a href="https://unsplash.com/photos/wCWhackqiQE">David Schap</a>)</p>
                    </figcaption>
                </figure>
            </div><!-- /end .figure.figure-article-lead -->

I SEE YOU FIGCAPTION

CUE DARTH VADER THEME

kissane commented 7 years ago

My assumption here, @ryanpitts, is that including our passé captions-as-paragraphs-with-classes captions in the caption-style CSS rule that is currently making figcaption elements look so nice will do the trick.

Does that seem true?

beep commented 7 years ago

What I might suggest is doubling up the selectors a bit, to cover old and new image styles. So, for example:

.figure,
.image-full-width-wrapper {
    font-family: map-get( $fonts, "serif" );;
    margin-bottom: 2em;
}
.wf-active .figure,
.wf-active .image-full-width-wrapper {
    font-family: map-get( $fonts, "serif" );;
}
.figure img,
.image-full-width-wrapper img {
    display: block;
    margin-bottom: 1em;
    width: 100%;
}
.figure figcaption,
.figure .caption,
.image-full-width-wrapper .caption {
    border-bottom: 1.125em solid #D8D8D8;    /* 18/16 */
    font-size: 16px;
    font-size: 1rem;
    padding-bottom: 1em;
}
.figure p,
.image-full-width-wrapper p {
    margin: 0;
}
.figure p + p,
.image-full-width-wrapper p + p {
    margin-top: 1em;
}

This might get a little ungainly, as I notice there’re a few other figure classes in that article. (.image-inset-left-wrapper) But barring a markup fix, that might be our best bet for now.

(hi this will be my last cssealioning, please don’t block me from yr githu

beep commented 7 years ago

or ooh, alternately, if you’re mainly worried about just the captions, you could just change .figure .caption to .caption, since we’re not really using that class elsewhere right now.

[exeunt]

ryanpitts commented 7 years ago

hi this will be my last cssealioning

no sir you have unlimited privileges

ryanpitts commented 7 years ago

also applying this, will push momentarily

ryanpitts commented 7 years ago

this CSS update is also on staging

kissane commented 7 years ago

Done done done done done done.