INN / umbrella-sfpublicpress

San Francisco Public Press
https://sfpublicpress.org/
GNU General Public License v2.0
1 stars 4 forks source link

Styling for related posts widget #57

Closed joshdarby closed 4 years ago

joshdarby commented 4 years ago

Changes

This pull request makes the following changes:

Desktop: Screen Shot 2020-02-11 at 4 35 46 PM

Mobile: screencapture-sfpp-test-2020-02-11-16_37_35

Why

For #31

Testing/Questions

Features that this PR affects:

Questions that need to be answered before merging:

Steps to test this PR:

  1. Add the Largo Related Posts widget to the article bottom widget area with these settings:
    • Title: Related Stories
    • Posts: 3
    • Show byline: yes
    • Widget title link: #
  2. View an article on desktop and mobile and make sure the styling matches the designs
benlk commented 4 years ago

I don't have a better idea of how to make sure we get the right image, short of patching https://github.com/INN/largo/issues/1769 in Largo. Here's the code that outputs that image: https://github.com/INN/largo/blob/1744bbc7ae99f501129d158c3b585cddf6b5409e/inc/widgets/largo-related-posts.php#L50-L65

With this we can center the smaller images:

ul.related li > a:first-child {
    text-align: center;
}
.widget.largo-related-posts ul.related li .wp-post-image {
    margin: 0 auto;
    width: unset;
}

Screen Shot 2020-02-11 at 17 43 37

But on mobile it looks even weirder to have the little centered images:

Screen Shot 2020-02-11 at 17 43 04

On small viewports, should we change to a vertical list of posts with left-aligned thumbnails?

Screen Shot 2020-02-11 at 17 44 55

joshdarby commented 4 years ago

@MirandaEcho @kaylima Any input on this? Are smaller, centered images ok here?

MirandaEcho commented 4 years ago

@MirandaEcho @kaylima Any input on this? Are smaller, centered images ok here?

@joshdarby - for mobile or desktop? On mobile, how much longer would it be to do the left-aligned ones? I agree the centered small looks a bit goofy on mobile. What was the original mobile design for it?

benlk commented 4 years ago

The small-viewport styles are a matter of commenting out styles added in this PR. Should be solvable by moving the added layout styles within a media query.

joshdarby commented 4 years ago

for mobile or desktop?

@MirandaEcho For desktop, and then the left aligned on mobile. Like @benlk said, the left aligned for mobile shouldn't take very long. We could even left align it on desktop, too, if you think that looks better than the small, centered image.

MirandaEcho commented 4 years ago

Original mobile designs had a center full width (on mobile) image, so that's fine.

joshdarby commented 4 years ago

Original mobile designs had a center full width (on mobile) image, so that's fine.

@MirandaEcho Just to be clear, here's what these proposed changes would look like.

Desktop (note image is not full width of story container): Screen Shot 2020-02-11 at 17 43 37

Mobile: Screen Shot 2020-02-11 at 17 44 55

kaylima commented 4 years ago

@MirandaEcho raising a flag: the byline name and title aren't easily readable. we may want to get ahead of this before client review

joshdarby commented 4 years ago

@MirandaEcho said she's fine with the smaller, centered image on desktop and left aligned on mobile.

joshdarby commented 4 years ago

https://github.com/INN/umbrella-sfpublicpress/pull/57/commits/0bd6bfbb71fd4c528393226ebb29a7de1b61bcdd implements the center aligned images on both desktop and mobile. We can't left align the image on mobile because that would require removing display: flex from the parent li, which we need in order keep the correct order of elements for each post.

Desktop: Screen Shot 2020-02-14 at 2 20 33 PM

Mobile: Screen Shot 2020-02-14 at 2 20 22 PM

benlk commented 4 years ago

Can we use the align-self property to change where the image is positioned within the flex container?