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

RichText changes and custom blocks backwards compatibility for WP 5.3 #17405

Closed
afercia opened this issue Sep 11, 2019 · 10 comments · Fixed by #17439 or #17451
Closed

RichText changes and custom blocks backwards compatibility for WP 5.3 #17405

afercia opened this issue Sep 11, 2019 · 10 comments · Fixed by #17439 or #17451
Assignees
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Priority] High Used to indicate top priority items that need quick attention [Status] In Progress Tracking issues with work in progress

Comments

@afercia
Copy link
Contributor

afercia commented Sep 11, 2019

Describe the bug
Recent changes related to the RichText component (tested in Gutenberg 6.4.0) seem to break backwards compatibility with custom blocks that work on WordPress 5.2 (not sure what the Gutenberg version is there).

To reproduce

  • on WordPress trunk 5.3 alpha
  • install latest Yoast SEO 12.0
  • create a post and add a "How-to" block
  • see the block UI works as expected:
    • clicking the 3 editable fields (actually they're 3 RichText) sets focus on the fields
    • the placeholder stays visible because the block uses keepPlaceholderOnFocus={ true }
    • when clicking the "step title" or the "step description", additional buttons appear within the UI
    • screenshot:

Screenshot 2019-09-11 at 15 18 48

  • activate the Gutenberg plugin: I tested latest master 6.4.0 50fb29b
  • click multiple times on the editable fields and see the fields area is only partially clickable, and sometimes clicking doesn't set focus on the fields
  • click the "step title" or the "step description": the additional buttons UI doesn't appear

Expected behavior
The custom blocks to work in the same way they work with the previous Gutenberg version.

Additional context

To my understanding, there are a few potential breaking changes in Gutenberg 6.4.0. Briefly:

The RichText is now split in two components:
#15212

There will be two RichText components in each package, where the one in the block-editor package wraps the one in the rich-text package and adds block context.

Also, there are several renamed props marked as “unstable”, for example isSelected is now __unstableIsSelected (besides unstableOnFocus).

There's a few temporary unstable props added that aid this wrapping, but in the future these props should be stabilised. Some props might disappears, some need to be renamed for the "bare" RichText component.

Looks like there's no backwards compatibility with custom blocks that make use of the RichText component out of the way Gutenberg uses it. I do realise Yoast SEO implementation is "custom" but that usage of RichText used to be legitimate and now it breaks on Gutenberg 6.4.0. There may be other potential breakages with other plugins as well.

The Yoast SEO blocks can be explored at https://github.com/Yoast/wordpress-seo/tree/trunk/js/src/structured-data-blocks

Also, the keepPlaceholderOnFocus prop was removed. This removal is undocumented so far. See #16733

Worth also reminding that wp.editor.RichText is now deprecated and wp.blockEditor.RichText should be used instead. When developers switch to wp.blockEditor.RichText they will get the new RichText which brings in props with different names. The old props names should be translated to the new names in some way: this specific point was mentioned by @gziolo

This issue was discussed during today's weekly editor meeting on Slack, see https://wordpress.slack.com/archives/C02QB2JS7/p1568208422122500

Agreed with the Gutenberg team to create a new issue to ensure backwards compatibility.

@afercia afercia added Backwards Compatibility Issues or PRs that impact backwards compatability [Priority] High Used to indicate top priority items that need quick attention labels Sep 11, 2019
@afercia
Copy link
Contributor Author

afercia commented Sep 11, 2019

Raising the priority as I'd tend to think these backwards compatibility concerns should be double checked in time for the WordPress 5.3 Beta phase, which is in a few weeks.

@ellatrix
Copy link
Member

  1. wp.blockEditor.RichText has been introduced a while time ago (by @youknowriad I believe) and has nothing to do with the RichText component abstraction, which you can find in the rich-text package. That component is not meant to be used in blocks, but is used by the wp.blockEditor.RichText internally.
  2. isSelected has not been renamed. It should still be there.

When developers switch to wp.blockEditor.RichText they will get the new RichText which brings in props with different names.

@afercia Which props are you talking about?

@ellatrix
Copy link
Member

I believe this issue can be closed with the merge of #17439.

@afercia
Copy link
Contributor Author

afercia commented Sep 16, 2019

@ellatrix et all, thanks for working on this. Unfortunately, seems to me not all the issues are solved.

I'm pretty confused by all these changes and by the fix in #17439 and I'm not sure I can follow. I'd also like to kindly invite the team to consider that from a plugin developers perspective, the effort to keep track of this kind of changes on Gutenberg master to try to prevent potential breakages when new versions will be shipped with a WordPress release is really not sustainable.

#17439 partially solves the issues on our custom blocks. However, it doesn't fix focus management which is still completely broken. When tabbing or clicking on our custom blocks, focus gets moved between blocks in an infinite loop. See the animated Gif on #17421 (comment)

I'm not really sure setFocusedElement is the only reason. The new navigation / edit mode might play a role. Regardless, our blocks work on WordPress 5.2 and break with the current Gutenberg plugin. From our perspective, this is really alarming because it means our blocks will break on WordPress 5.3.

Our blocks import the RichText that is available on WP 5.2.:

const { RichText } = window.wp.editor;

I'm not even sure which RichText is that. Honestly, as plugin developers we shouldn't even care and we expect our blocks to just work as in WordPress 5.2.

Of course, we saw the deprecation warning. We'd just like to know what we are supposed to do to make sure our blocks don't break in the Gutenberg plugin and in WordPress 5.3 when it will be released 🙂

Also, how we can do that keeping backwards compatibility with WordPress 5.2., 5.1, and 5.0.

Which props are you talking about?

I'm very confused also in this regard. I may be wrong but see isSelected is now __unstableIsSelected. Does this mean it can be removed at any time in the future without notices, as the policy for unstable props states? Not sure about the other unstable ones.

Reopening, pending feedback.

@afercia afercia reopened this Sep 16, 2019
@ellatrix
Copy link
Member

@afercia isSelected has not been removed nor deprecated. It should still work. It's used in the core image block for example. Also in the gallery block. It is true that it maps internally to __unstableIsSelected for wp.richText.RichText. But blocks would never use that component directly. They should use the wrapper component wp.blockEditor.RichText, which still has the isSelected as before.

I'll try to install Yoast SEO, but it would be great if an isolated text case could be provided to reproduce the selection issues.

@ellatrix
Copy link
Member

ellatrix commented Sep 16, 2019

I'm trying Yoast SEO + Gutenberg master and it all seems to work fine. I can fill in every field, I can tab through them etc. The only small issue is mentioned in #17439 is that, only on click when the field is empty, the caret is hidden. But the focus is still there and you can type. Of course we should fix this, and I'll look at it asap. But other than that, I don't see any issues. Would be good to have steps to reproduce if that's not all.

@afercia
Copy link
Contributor Author

afercia commented Sep 16, 2019

To reproduce:

  • add a "How-to" block and leave it empty
  • add a "FAQ" block and leave it empty
  • click within the editables: step title, step description, and then question
  • see focus gets moved automatically in a loop

Animated GIF:

loop 02

Same happens when tabbing through the blocks: animated GIF:

loop

Reproduced on:

  • WordPress 5.2.2
  • Yoast SEO 12.0
  • Gutenberg plugin 6.4.0 activated

and also on:

  • WordPress trunk 5.3. alpha
  • Yoast SEO trunk from the GitHub repo
  • Gutenberg plugin 6.4.0 activated

Disabling the Gutenberg plugin makes the Yoast SEO blocks focus management work again.

@youknowriad
Copy link
Contributor

Great job testing and finding all the regressions @afercia. This will ensure the next release to be more stable. Let's follow-up with other issues if we notice any other regression.

@afercia
Copy link
Contributor Author

afercia commented Sep 16, 2019

Thanks everyone. #17451 seems to fix the focus management issue.

Taking the opportunity to remind about the need for plugins to have a native way to manage focus on the RichText. See this old issue: #9740

@youknowriad
Copy link
Contributor

Right! I think focus management is possible right now in a very nice way using the selection state (actions and selectors). This needs to be better documented thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Priority] High Used to indicate top priority items that need quick attention [Status] In Progress Tracking issues with work in progress
Projects
None yet
4 participants