Skip to content
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

[LatestPosts] Fixes the excerpt length #20313

Merged
merged 7 commits into from
Mar 2, 2020

Conversation

draganescu
Copy link
Contributor

Description

Closes #20311

It is a bug introduced by #19669

How has this been tested?

Tested locally by:

  • add a post
  • add a latest posts block
  • set the block to show excerpt
  • set the limit of excerpt words to be 10
  • save the post
  • check that the rendering does not exceed 10 words in the excerpt

Types of changes

Introduces an excerpt_length filter in the LatestPosts block.

@draganescu draganescu added [Type] Bug An existing feature does not function as intended [Block] Latest Posts Affects the Latest Posts Block labels Feb 19, 2020
@draganescu draganescu changed the title adds a filter that sets the excerpt length to what was set in the editor Fixes the excerpt length on LatestPosts block Feb 19, 2020
@github-actions
Copy link

github-actions bot commented Feb 19, 2020

Size Change: +6.24 kB (0%)

Total Size: 865 kB

Filename Size Change
build/annotations/index.js 3.43 kB -4 B (0%)
build/api-fetch/index.js 3.39 kB +1 B
build/block-directory/index.js 6.02 kB +1 B
build/block-editor/index.js 105 kB +671 B (0%)
build/block-editor/style-rtl.css 10.5 kB +705 B (6%) 🔍
build/block-editor/style.css 10.5 kB +713 B (6%) 🔍
build/block-library/editor-rtl.css 7.4 kB -276 B (3%)
build/block-library/editor.css 7.4 kB -272 B (3%)
build/block-library/index.js 116 kB +3.81 kB (3%)
build/block-library/style-rtl.css 7.5 kB +22 B (0%)
build/block-library/style.css 7.5 kB +21 B (0%)
build/blocks/index.js 57.6 kB +8 B (0%)
build/components/index.js 191 kB +918 B (0%)
build/components/style-rtl.css 15.5 kB -529 B (3%)
build/components/style.css 15.5 kB -535 B (3%)
build/compose/index.js 5.76 kB -2 B (0%)
build/core-data/index.js 10.5 kB +5 B (0%)
build/data-controls/index.js 1.03 kB -4 B (0%)
build/data/index.js 8.22 kB -7 B (0%)
build/date/index.js 5.37 kB +5 B (0%)
build/deprecated/index.js 772 B +1 B
build/dom/index.js 3.06 kB -1 B
build/edit-post/index.js 90.9 kB +414 B (0%)
build/edit-post/style-rtl.css 8.53 kB -158 B (1%)
build/edit-post/style.css 8.53 kB -150 B (1%)
build/edit-site/index.js 4.63 kB +1.92 kB (41%) 🚨
build/edit-site/style-rtl.css 2.51 kB -108 B (4%)
build/edit-site/style.css 2.51 kB -108 B (4%)
build/edit-widgets/index.js 4.42 kB +68 B (1%)
build/edit-widgets/style-rtl.css 2.59 kB -208 B (8%)
build/edit-widgets/style.css 2.58 kB -207 B (8%)
build/editor/editor-styles-rtl.css 325 B -2 B (0%)
build/editor/editor-styles.css 327 B -1 B
build/editor/index.js 44.6 kB -501 B (1%)
build/editor/style-rtl.css 4.01 kB -115 B (2%)
build/editor/style.css 4 kB -111 B (2%)
build/element/index.js 4.45 kB -3 B (0%)
build/format-library/style-rtl.css 502 B +2 B (0%)
build/format-library/style.css 502 B +1 B
build/hooks/index.js 1.92 kB -1 B
build/i18n/index.js 3.48 kB +36 B (1%)
build/is-shallow-equal/index.js 710 B -1 B
build/keyboard-shortcuts/index.js 2.3 kB -2 B (0%)
build/keycodes/index.js 1.68 kB -1 B
build/list-reusable-blocks/style-rtl.css 226 B +11 B (4%)
build/list-reusable-blocks/style.css 226 B +10 B (4%)
build/media-utils/index.js 4.85 kB +5 B (0%)
build/notices/index.js 1.57 kB -1 B
build/nux/index.js 3.02 kB +4 B (0%)
build/plugins/index.js 2.54 kB +303 B (11%) ⚠️
build/primitives/index.js 1.49 kB -2 B (0%)
build/priority-queue/index.js 780 B -98 B (12%) 👏
build/redux-routine/index.js 2.84 kB -3 B (0%)
build/rich-text/index.js 14.3 kB -4 B (0%)
build/server-side-render/index.js 2.54 kB -2 B (0%)
build/url/index.js 4 kB +2 B (0%)
build/viewport/index.js 1.61 kB +2 B (0%)
build/warning/index.js 1.14 kB +1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.01 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/dom-ready/index.js 568 B 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.6 kB 0 B
build/html-entities/index.js 622 B 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we apply the trimming according to the defined length to this line instead?

			$trimmed_excerpt = get_the_excerpt( $post );

@draganescu
Copy link
Contributor Author

draganescu commented Feb 20, 2020

As discussed in the design channel on Slack we should stick to the following process:

  • blocks that work with excerpts, right now PostExcerpt and LatestPosts
  • should default to the global excerpt setting (set by the excerpt_length) filter
  • but have a local setting
  • which only affects their rendered content
  • but only generated excerpts
  • manual excerpts should be rendered integrally, independent of what the excerpt_length setting is.

This behavior is both consistent to how WordPress currently handles excerpts and also with what a user would expect from the system.

There are two new issues that spawn from this:

@draganescu draganescu changed the title Fixes the excerpt length on LatestPosts block [LatestPosts] Fixes the excerpt length Feb 26, 2020
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @draganescu, I think the reason we were using a filter was to make sure manual excerpts are respected but in my tests, while this PR makes the excerpts not trimmed on the frontend, it seems manual excerpts are not respected on the editor.

I created a post with the following manual except: aaaaa1 bbbb2 cccc3 dddd4 eeee5 ffff6 gggg7 hhhhh8 iiiiii9 jjjjjj10 llllll11 mmmmmm12 nnnnnn13, and published it.

I created another post with the latest posts block with except length of 10. The excerpt displayed as aaaaa1 bbbb2 cccc3 dddd4 eeee5 ffff6 gggg7 hhhhh8 iiiiii9 jjjjjj10 ... Read more instead of showing the complete excerpt I wrote on the editor.

packages/block-library/src/latest-posts/index.php Outdated Show resolved Hide resolved
packages/block-library/src/latest-posts/index.php Outdated Show resolved Hide resolved
packages/block-library/src/latest-posts/index.php Outdated Show resolved Hide resolved
packages/block-library/src/latest-posts/index.php Outdated Show resolved Hide resolved
packages/block-library/src/latest-posts/index.php Outdated Show resolved Hide resolved
packages/block-library/src/latest-posts/index.php Outdated Show resolved Hide resolved
packages/block-library/src/latest-posts/index.php Outdated Show resolved Hide resolved
packages/block-library/src/latest-posts/index.php Outdated Show resolved Hide resolved
…e remove and is not affecting theme set length
@draganescu
Copy link
Contributor Author

@aduth regarding the use of anonymous functions as filters I tested the implementation of filtering excerpt_length in the latest post block with an anonymous function assigned to a variable:

	$block_core_latest_posts_excerpt_length = function( $length ) use ( $attributes ) {
		if ( ! empty( $attributes['excerptLength'] ) ) {
			return $attributes['excerptLength'];
		}
		return $length;
	};
	add_filter( 'excerpt_length', $block_core_latest_posts_excerpt_length, 999 );

then removing it with

remove_filter( 'excerpt_length', $block_core_latest_posts_excerpt_length );

then in single.php I created at the end (after displaying the single post) a new loop:

$my_query = new WP_Query( $args );
if ( $my_query->have_posts() ) {
    while ( $my_query->have_posts() ) {
        $my_query->the_post();
		echo '<hr>';
		echo get_the_excerpt();
		echo '<hr>';
    }
}

and these posts had their excerpt stuck to the length set by the block latest posts contained in the single post. So it means we can't do it any other way but with named functions so that we can remove them after we render the blocks.

Using a global reference and a named function the same test reverts OK to the default WP excerpt length.

@draganescu
Copy link
Contributor Author

@jorgefilipecosta there is another PR that fixes the block's edit to respect manual excerpts: #20432

Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@draganescu draganescu merged commit e01b562 into master Mar 2, 2020
@draganescu draganescu deleted the fix/excerpt-length-latest-posts branch March 2, 2020 19:25
@github-actions github-actions bot added this to the Gutenberg 7.7 milestone Mar 2, 2020
@jorgefilipecosta jorgefilipecosta added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Mar 2, 2020
jorgefilipecosta pushed a commit that referenced this pull request Mar 2, 2020
* adds a filter that sets the excerpt length to what was set in the editor

* adds named function as excerpt filter and removes after use

* adds PHP doc to the new filter callback and global variable

* default to using an anonymous function since it appears that it can be remove and is not affecting theme set length

* revert to named function for excerpt length filter

* proper function names for Guternberg namespace

* lower default priority for the latest posts excerpt length filter
jorgefilipecosta pushed a commit that referenced this pull request Mar 2, 2020
* adds a filter that sets the excerpt length to what was set in the editor

* adds named function as excerpt filter and removes after use

* adds PHP doc to the new filter callback and global variable

* default to using an anonymous function since it appears that it can be remove and is not affecting theme set length

* revert to named function for excerpt length filter

* proper function names for Guternberg namespace

* lower default priority for the latest posts excerpt length filter
@jorgefilipecosta jorgefilipecosta removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Mar 3, 2020
@raQai
Copy link

raQai commented Apr 22, 2020

Just because customers flooded my e-mail and didn't know this was design by choice: is there any way to also trim manual excerpts in the block or do i have to edit the core files?

This behavior is both consistent to how WordPress currently handles excerpts and also with what a user would expect from the system

I can verify at least 12 of my customers do not agree after updating to 5.4. All of them expected the manual excerpt to be trimmed as well.

@draganescu
Copy link
Contributor Author

Howdy @raQai and thank you for the feedback!

This change to the LatestPosts block may have broken some fixed layouts, is this your customer's case as well?

It is important why would a manual excerpt be trimmed. How would this benefit your customers?

@raQai
Copy link

raQai commented Apr 24, 2020

I was finally able to check all the sites "affected" by this. Some of them used fixed layouts, some have been wondering because they used the "read more" link in form of a button to give better feedback to site visitors showing them "hey, you can continue reading here."

The main issue for most of them relates to the latter.
Relying on the users to understand that a fully written text might not be the full post (while also hoping for them to click on the link of the title) feels kind of strange.

As far as I can tell, there is no direct hook to append this button, is it?
Do i have to register my own block type then or can I somehow hook into the renderer?

@draganescu
Copy link
Contributor Author

draganescu commented Apr 29, 2020

Just to make sure I understand:

  • the problem is that:
    • in pages using the LatestPosts block
    • for posts that have manual excerpts
      • there is no read more link
      • and users don't know there is more?

@raQai
Copy link

raQai commented Apr 29, 2020

@draganescu correct.

A simple checkbox to allow to "always display read more link" would solve this issue. That's what I am currently working on but since I never worked on custom gutenberg blocks before, I first have to dig into the documentation :)

@draganescu
Copy link
Contributor Author

draganescu commented Apr 30, 2020

I'll open an issue for this. Thank you! The problem is some themes add their own link and you end up with two :) so we need a way to know that the theme adds its own read more link.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Latest Posts Affects the Latest Posts Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Latest Posts block: Excerpt doesn't respect the excerptLength attribute (WP 5.4 Beta 2)
5 participants