Make WordPress Core

Opened 9 months ago

Closed 9 months ago

Last modified 9 months ago

#59681 closed defect (bug) (fixed)

Regression: Fix layout support to be compatible with enhanced pagination

Reported by: luisherranz's profile luisherranz Owned by: hellofromtonya's profile hellofromTonya
Milestone: 6.4 Priority: normal
Severity: normal Version:
Component: Editor Keywords: gutenberg-merge has-patch has-unit-tests commit dev-reviewed
Focuses: Cc:

Description (last modified by hellofromTonya)

This ticket tracks the merging of PHP files for the following Gutenberg changes:

https://github.com/WordPress/gutenberg/pull/55416

This is fixing a regression introduced in 6.4 with the new enhanced pagination feature.

Change History (21)

This ticket was mentioned in PR #5528 on WordPress/wordpress-develop by @luisherranz.


9 months ago
#1

  • Keywords has-patch added

This is a backport PR for WordPress 6.4 that includes the following PHP Gutenberg changes:

Trac ticket: https://core.trac.wordpress.org/ticket/59681

@cbravobernal commented on PR #5528:


9 months ago
#2

Hi there!

I applied all changes requested. Thanks for the review @tellthemachines @costdev

@cbravobernal commented on PR #5528:


9 months ago
#3

Code updated with changes requested. Thanks for your reviews @tellthemachines , @costdev ! 🙇

@luisherranz commented on PR #5528:


9 months ago
#4

This looks much better now. Thanks, @c4rl0sbr4v0 and @costdev! 🙏

#5 @cbravobernal
9 months ago

  • Milestone changed from Awaiting Review to 6.4

#6 @hellofromTonya
9 months ago

This change is the 2nd half of fix for the Gutenberg issue 55230. The first half is in ticket #59694. Both tickets should be considered together for whether they should or shouldn't be backported to the 6.4 branch, as together they solve the issue with enhanced pagination in the Query block.

As I noted on #59694, for the bugfix to be backported to the 6.4 branch to ship in 6.4 RC2 or 3, it needs to resolve a regression or break/error introduced during the 6.4 cycle, i.e. per the commit guidelines during the release candidate phase:

Regressions: bugs that have been introduced during the WordPress 6.4 development cycle, either to existing or new features.

Some notes:

@cbravobernal commented on PR #5528:


9 months ago
#7

Patch Report

Patch tested: https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/5528.diff

How to read this report:

🐞 <= Bug occurs.
✅ <= Behavior is expected.
❌ <= Behavior is NOT expected.

Environment

  • OS: macOS 14.0
  • Web Server: Nginx
  • macOS 14.0
  • PHP 8.2.11
  • 6.4-RC1-56475-src
  • Firefox 118.0.2

Steps to Reproduce

  1. With WordPress 6.4 installed
  2. Create 7 posts, configure to have 3 per page.
  3. On the site editor, in home page, TT4, add pagination to the Query Loop, set it to inherit the template settings, activate "Enhanced Pagination feature). Make sure your layout is three posts in a row.
  4. 🐞 Check that on page three, the last post layout is broken (full width instead of 1/3)

Steps to Test Patch

  1. 🛠 Apply the patch.
  2. Refresh the home page.
  3. ✅ Check that the layout is correct when navigating to the last page.
  4. ✅ Check that the navigation happened without a page reload.

Test Results

  • ✅ Issue is reproducible.
  • ✅ Issue resolved with patch.

Screenshots

Before patch:
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/37012961/2eb469a2-2b83-4808-8c2a-908f961648da

After patch:
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/37012961/0814e120-2c74-4bbd-80c1-c143e220ea1f

This ticket was mentioned in Slack in #core by cbravobernal. View the logs.


9 months ago

#9 @hellofromTonya
9 months ago

In discussions and reviews, this is fixing a 6.4 regression introduced with the new enhanced pagination feature. Thus it is okay to be fixed in the 6.4 RC cycle.

#10 @hellofromTonya
9 months ago

  • Description modified (diff)

#11 @hellofromTonya
9 months ago

  • Summary changed from Make layout support compatible with enhanced pagination to Regression: Fix layout support to be compatible with enhanced pagination

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


9 months ago

@hellofromTonya commented on PR #5528:


9 months ago
#13

@aaronjorbin @costdev all feedback is now addressed. Can you re-review please?

#14 @hellofromTonya
9 months ago

  • Keywords has-unit-tests commit added
  • Owner set to hellofromTonya
  • Status changed from new to reviewing

#15 @jorbin
9 months ago

  • Keywords dev-reviewed added

This looks good. Marking as dev-reviewed to endorse committing to trunk and immediately backporting.

@Bernhard Reiter commented on PR #5528:


9 months ago
#16

Thank you everyone! Luis has asked me to commit this, which I'll promptly do 😊

#17 @Bernhard Reiter
9 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 56994:

Blocks: Fix layout support to be compatible with enhanced pagination.

Make layout support compatible with enhanced pagination by ensuring that generated class names are stable across pagination, even when the number of rendered posts is different.

With the previous implementation of enhanced pagination, the CSS corresponding to each block was not detected. Therefore, for enhanced pagination to work correctly, the CSS of the blocks present in the Post Template must be stable on all pages.

The number of posts rendered by the Query block is always the same, except in the last page, where it can be only a fraction. If any of the blocks rendered by the Post Template used the wp_unique_id function, the ID (which is incremental) would have been different than in the previous pages and the class names would have varied.

This is remediated by this changeset by replacing the usage of wp_unique_id in the layout support (which is used by the Query block) with an implementation that uses IDs that are incremental only for that block. That way, the generated class names are never affected by the number of times wp_unique_id runs.

Props luisherranz, andrewserong, isabel_brison, costdev, mukesh27, cbravobernal, hellofromTonya, jorbin.
Fixes #59681.

#18 @Bernhard Reiter
9 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backporting to the 6.4 branch.

#19 @Bernhard Reiter
9 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 56995:

Blocks: Fix layout support to be compatible with enhanced pagination.

Make layout support compatible with enhanced pagination by ensuring that generated class names are stable across pagination, even when the number of rendered posts is different.

With the previous implementation of enhanced pagination, the CSS corresponding to each block was not detected. Therefore, for enhanced pagination to work correctly, the CSS of the blocks present in the Post Template must be stable on all pages.

The number of posts rendered by the Query block is always the same, except in the last page, where it can be only a fraction. If any of the blocks rendered by the Post Template used the wp_unique_id function, the ID (which is incremental) would have been different than in the previous pages and the class names would have varied.

This is remediated by this changeset by replacing the usage of wp_unique_id in the layout support (which is used by the Query block) with an implementation that uses IDs that are incremental only for that block. That way, the generated class names are never affected by the number of times wp_unique_id runs.

Props luisherranz, andrewserong, isabel_brison, costdev, mukesh27, cbravobernal, hellofromTonya, jorbin.
Merges [56994] to the 6.4 branch.
Fixes #59681.

@Bernhard Reiter commented on PR #5528:


9 months ago
#20

Committed to Core trunk in `[56994] and to the 6.4 branch in [https://core.trac.wordpress.org/changeset/56995 [56995]`].

@luisherranz commented on PR #5528:


9 months ago
#21

Thank you all for your help getting this to the finish line! @ockham, @c4rl0sbr4v0, @hellofromtonya, @aaronjorbin and @costdev 🙏

Note: See TracTickets for help on using tickets.