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

Experimental: Fix TinyMCE removal for heartbeat requests #52935

Merged
merged 3 commits into from
Jul 26, 2023

Conversation

tyxla
Copy link
Member

@tyxla tyxla commented Jul 25, 2023

What?

This PR fixes warnings that are thrown for heartbeat requests when the TinyMCE removal experiment is turned on.

The warnings are easy to see in your browser console if you have the Query Monitor plugin installed.

Why?

We're just fixing some warnings:

Screenshot 2023-07-25 at 15 13 21

Found while working on #52811.

How?

We're moving the $current_post declaration outside the condition that declares it and moving the condition above. That way, the code that uses $current_post will actually be reached only when necessary.

Testing Instructions

  • Install the Query Monitor plugin.
  • Open the post editor for an existing post.
  • Wait for a minute to witness a heartbeat XHR request to run.
  • Verify there are no longer warnings.
  • Try the same as above but for a custom post type.
  • Verify test instructions in Try: Aggressive TinyMCE deprecation #50387 still work well.

Testing Instructions for Keyboard

None.

Screenshots or screencast

None.

@tyxla tyxla added [Type] Enhancement A suggestion for improvement. [Package] Block library /packages/block-library [Block] Classic Affects the Classic Editor Block [Type] Experimental Experimental feature or API. labels Jul 25, 2023
@tyxla tyxla self-assigned this Jul 25, 2023
@tyxla tyxla requested a review from spacedmonkey as a code owner July 25, 2023 12:18
@github-actions
Copy link

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/experimental/disable-tinymce.php

@github-actions
Copy link

github-actions bot commented Jul 25, 2023

Flaky tests detected in 2931e45.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5657072477
📝 Reported issues:

@skorasaurus
Copy link
Member

This PR removes PHP warnings that had appeared in the background (that I mention at #52936 (comment)) but...

The classic block appears in the block inserter and I can insert a classic block, add some text content and save a new post, even with the post type of post.

@tyxla
Copy link
Member Author

tyxla commented Jul 26, 2023

The classic block appears in the block inserter and I can insert a classic block, add some text content and save a new post, even with the post type of post.

Odd, I'm unable to reproduce. Is there a chance you checked this PR out and didn't trigger a fresh Gutenberg build? I wonder if we can get a second opinion since I really want to land this and resolve the PHP warnings that are appearing and blocking testing of the rest of the PRs that address #52811.

@Mamaduka
Copy link
Member

I should've checked this PR before filing my own 😅 Going to test it now.

$content = $current_post->post_content;
// Bail if for some reason the post isn't found.
$current_post = get_post( intval( $_GET['post'] ) );
if ( ! $current_post || is_wp_error( $current_post ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

We can remove the is_wp_error check here. See https://developer.wordpress.org/reference/functions/get_post/#return

Copy link
Member Author

Choose a reason for hiding this comment

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

I was almost sure that WP_Post could return a WP_Error, but I must have misremembered! Anyway, you're right, fixed in 6bb68ee

Copy link
Member

@jsnajdr jsnajdr left a comment

Choose a reason for hiding this comment

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

lgtm 👍

Post IDs are supposed to be non-negative integers.

Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com>
Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

Resolves the issue with PHP warnings.

I'm also unable to reproduce the inserter behavior that @skorasaurus reported. The Classic block isn't available in the inserter for me.

@tyxla
Copy link
Member Author

tyxla commented Jul 26, 2023

Thank you both 🙌 Will ship ASAP to resolve the warnings.

@tyxla tyxla enabled auto-merge (squash) July 26, 2023 08:02
@tyxla tyxla merged commit a78a632 into trunk Jul 26, 2023
49 checks passed
@tyxla tyxla deleted the fix/tinymce-heartbeat-requests branch July 26, 2023 08:31
@github-actions github-actions bot added this to the Gutenberg 16.4 milestone Jul 26, 2023
@SiobhyB SiobhyB added the Needs PHP backport Needs PHP backport to Core label Sep 5, 2023
@github-actions
Copy link

github-actions bot commented Sep 5, 2023

Warning: Type of PR label error

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

  • Type-related labels to choose from: [Type] Accessibility (a11y), [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core.
  • Labels found: [Type] Enhancement, [Package] Block library, [Block] Classic, [Type] Experimental.

Read more about Type labels in Gutenberg.

@mikachan mikachan removed the Needs PHP backport Needs PHP backport to Core label Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Classic Affects the Classic Editor Block [Package] Block library /packages/block-library [Type] Enhancement A suggestion for improvement. [Type] Experimental Experimental feature or API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants