Discover: show full post excerpt in stream #9410
Conversation
bluefuton
added
[Feature] Reader
bluefuton
added
[Status] In Progress
and removed
[Status] Needs Review
|
bluefuton
force-pushed
the
update/reader/discover-excerpt-show-all-for-picks
branch
from
c6a32ce to
7a0e790
Compare
bluefuton
added
[Status] Needs Review
bluefuton
changed the title
| .reader-post-card.card:not(.is-showing-entire-excerpt) { | ||
| .reader-post-card__excerpt { | ||
| overflow: hidden; | ||
| max-height: 15px * 1.6 * 3; |
| @@ -43,6 +43,10 @@ export default function createBetterExcerpt( post ) { | |||
| return post; | |||
| } | |||
|
|
|||
| // Create standard excerpt for Discover | |||
| post.excerpt_no_html = formatExcerpt( post.excerpt ); | |||
There was a problem hiding this comment.
i thought the intent was to not mess with the excerpt at all?
There was a problem hiding this comment.
^ That's correct (not messing with the excerpt).
There was a problem hiding this comment.
The excerpt sometimes comes wrapped in a <p>, which is all I'm wanting to strip here.
If we don't remove this wrapping paragraph, we'd have to display the excerpt using dangerouslySetInnerHTML, which feels wrong.
| .reader-post-card.card:not(.is-showing-entire-excerpt) { | ||
| .reader-post-card__excerpt { | ||
| overflow: hidden; | ||
| max-height: 15px * 1.6 * 3; |
There was a problem hiding this comment.
I'm curious what the advantage is of using this since it isn't relative and we end up with a fixed value anyway?
There was a problem hiding this comment.
I like it just because it's explanatory and less magical. 72px wouldn't tell me why 72.
There was a problem hiding this comment.
My question then is why 15px? 😄
If we need the context here then maybe we could use some variables?
or maybe a comment with the meaning for each multiplier? like
max-height: 15px * 1.6 * 3; // Some value * what ever 1.6 means * something else
| originalPost: PropTypes.object // used for Discover only | ||
| originalPost: PropTypes.object, // used for Discover only | ||
| showEntireExcerpt: PropTypes.bool, | ||
| excerptAttribute: PropTypes.string |
There was a problem hiding this comment.
If there are only two possible values I wonder if it would be overkill here to make this PropTypes.oneOf( [ 'excerpt_no_html', 'better_excerpt_no_html' ] ). We could even go further and use constants throughout for those values.
There was a problem hiding this comment.
Or perhaps just a boolean useBetterExcerpt?
There was a problem hiding this comment.
Changed in 88ea09b.
bluefuton
force-pushed
the
update/reader/discover-excerpt-show-all-for-picks
branch
from
4ebe48e to
88ea09b
Compare
| @@ -22,8 +27,7 @@ export function isDiscoverPost( post ) { | |||
| } | |||
|
|
|||
| export function isDiscoverSitePick( post ) { | |||
| const metaData = get( post, 'discover_metadata.discover_fp_post_formats' ); | |||
| return !! ( metaData && find( metaData, { slug: 'site-pick' } ) ); | |||
| return hasDiscoverSlug( post, 'site-pick' ); | |||
|
code LGTM 🚢 Testing done:
|
samouri
added
[Status] Ready to Merge
and removed
[Status] Needs Review
bluefuton
deleted the
update/reader/discover-excerpt-show-all-for-picks
branch
For Discover posts, show the entire
post.excerptin the stream.A normal post (clamped at 3 lines):
A Discover pick (shows entire excerpt):
Part of #9096.
cc @jancavan for CSS sanity check.