-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Cherry-picked bug fixes for WP 6.5.3 #61299
Conversation
Co-authored-by: ntsekouras <[email protected]> Co-authored-by: jorgefilipecosta <[email protected]> Co-authored-by: Mamaduka <[email protected]>
* Make footer a fixed height; add margin to modal content * Remove Spacer from bottom of Library tab content * Add variable for footer height size Co-authored-by: mikachan <[email protected]> Co-authored-by: creativecoder <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: jordesign <[email protected]> Co-authored-by: matiasbenedetto <[email protected]>
…th `data-wp-on`. (#60661) * Allow multiple event handlers for the same type * Add simple test Co-authored-by: DAreRodz <[email protected]> Co-authored-by: felixarntz <[email protected]> Co-authored-by: luisherranz <[email protected]>
… deeply nested lists (#60845) Co-authored-by: andrewserong <[email protected]> Co-authored-by: ramonjd <[email protected]> Co-authored-by: diggeddy <[email protected]>
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
These three PRs are marked for backport but did not cherry-pick cleanly, I'm working on resolving the conflicts before pushing them to this branch: |
Size Change: +206 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
Co-authored-by: andrewserong <[email protected]> Co-authored-by: ramonjd <[email protected]>
@andrewserong @tellthemachines Any chance you can help cherry pick 59a4104 (#60489) into this branch? There are several conflicts in |
* Layout: Always add semantic classes * Default to undefined * Feedback Co-authored-by: Mamaduka <[email protected]> Co-authored-by: tellthemachines <[email protected]> Co-authored-by: andrewserong <[email protected]> Co-authored-by: colorful-tones <[email protected]> Co-authored-by: ellatrix <[email protected]> Co-authored-by: davecpage <[email protected]>
… theme.json (#60764) Co-authored-by: tellthemachines <[email protected]> Co-authored-by: oandregal <[email protected]> Co-authored-by: andrewserong <[email protected]> Co-authored-by: ramonjd <[email protected]> Co-authored-by: creativecoder <[email protected]>
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/class-wp-theme-json-gutenberg.php ❔ phpunit/class-wp-theme-json-test.php |
Oh, weird that #60764 was successfully cherry-picked, because it also changes the tests. Maybe the task isn't doing them in the correct order? To avoid conflicts, commits need to be cherry-picked in the order they were merged to trunk; I thought we'd fixed the task to do that 🤔 I'll look into it! |
In any case, that commit shouldn't block the package publishing, because it's a PHP only change (it's been committed to the release branch in core already) so feel free to go ahead with it if you're pressed for time! That commit should still be cherry-picked for completeness but it's not needed here for 6.5.3 package publishing purposes. |
I've manually cherry-picked the following onto this branch: Layout: Always add semantic classes #60668 (26e35c7) @Mamaduka @tellthemachines @andrewserong , I could use your help testing to make sure the fix is working correctly in this branch, as I had to manually reconcile merge conflicts. Commit to this branch is here: fd26a36 Don't output base flow and constrained layout rules on themes without theme.json #60764 (cc8fce2) The fix came in cleanly, but the test changes in @tellthemachines @oandregal @andrewserong Please take a look and make sure this fix and the test expectations are working as expected. The commit to this branch is here: 84ac9e3 |
Thanks for taking a look @tellthemachines
I had to manually resolve a conflict in the test file for #60764 as well, so that may also need a look.
The cherry pick script seems to be working correctly, but if a commit fails the cherry pick, it just gets skipped, so that could be a problem here. But the first PR cherry picked here was #60489, and that one failed right away. I think it's a combination of the multiple changes to
Thanks for pointing that out, I'll keep it in mind as an option. It looks like the same is true for #60764, as well. The plan is to create the WP 6.5.3 release candidate on May 2nd at 17:00 UTC. |
@DAreRodz @luisherranz @felixarntz @gziolo It appears that an e2e test in I'm not sure if it's related to the change coming in from #60661, or something else. Could you take a look? |
Yeah that sounds right; any other changes to the theme json class in trunk would have caused diffs in those tests. In my experience the best way to fix them is cherry-picking just the non-test changes from the related PRs, then running the tests locally and updating the expected test strings to whatever the actual output is. Incidentally, I opened #60842 to discuss improvements to that test suite, so any thoughts you might have to add there would be welcome! |
@@ -657,7 +657,7 @@ test.describe( 'Post Editor Performance', () => { | |||
|
|||
const startTime = performance.now(); | |||
|
|||
await page.getByRole( 'button', { name: 'Test' } ).click(); | |||
await page.getByText( 'Test' ).click(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This locator was causing the failure. The element is button on wp/6.5
branch, but after recent refactoring it's tab
on trunk. See #60257.
I think it's okay to ignore React Native and Interactivity e2e test failures. Latter is known to be flaky on this branch - #60241 (comment). |
@tellthemachines Awesome, thanks for testing.
I'll go ahead and manually cherry-pick #60489, using the expected test results from the failed tests to update and get the tests passing. That feels backwards, but given this branch is effectively only for JS package releases, and the failing tests are all for PHP changes that are separately tested by core unit tests, it seems acceptable. |
…ide sizes if no layout sizes exist (#60489) * Layout: Skip outputting base layout rules that reference content or wide sizes if no content or wide sizes exist * Update tests
I've finished cherry picking "#60489 – Layout: Skip outputting base layout rules that reference content or wide sizes if no layout sizes exist" to this branch. I was able to use the git extension in VS Code to see that the changes to
This seems to line up with the intent of the PR, so I think the merge conflicts are resolved correctly. |
This is now ready to merge (but still needs an approval). I'm overlooking the React Native test failures based on this comment, all other CI is passing. |
@WordPress/gutenberg-core - Is there someone that can please review this so that the bug fixes can be included in 6.5.3? |
Includes
#60489 – Layout: Skip outputting base layout rules that reference content or wide sizes if no layout sizes exist
#60620 – Fix inserter pattern pagination focus loss
#60608 – Fix static posts page setting resolved template
#60641 – Font Library: Fix modal scrollbar
#60661 – Interactivity API: Allow multiple event handlers for the same type with
data-wp-on
.#60668 – Layout: Always add semantic classes
#60845 – List View: Fix stuck dragging mode in UI in Firefox when dealing with deeply nested lists
#60764 – Don't output base flow and constrained layout rules on themes without theme.json
Also
#60686 - PHP unit test workflow: Try removing 7.0 and 7.1 to get CI tests passing