-
Notifications
You must be signed in to change notification settings - Fork 297
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
Use inline styles for Sign in with Google Gutenberg block #10265
Conversation
Build files for 92086fd have been deleted. |
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.
Thanks @tofumatt! This works great on the latest WP but I found issues (unrelated to this change) that we should address before rolling out this block to everyone. More detail below, but we should limit the block's availability to WP 5.8+.
@@ -1,11 +1,11 @@ | |||
{ | |||
"$schema": "https://schemas.wp.org/trunk/block.json", | |||
"apiVersion": 3, | |||
"apiVersion": 2, |
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.
v2 was released in WP 5.6 with the addition of useBlockProps
– I'm a little concerned that this may break things for folks on those older versions as this will be available to everyone. I'm not sure core has the concept of the API version at that time to prevent it from loading, but it actually errors when registered in JS right now due to a missing title
.

Similarly, there is a notice raised on the backend due to the use of the block.json in the registration (I assume) as it's complaining about the lack of a prefix
Notice: WP_Block_Type_Registry::register was called incorrectly. Block type names must contain a namespace prefix. Example: my-plugin/my-custom-block-type Please see [Debugging in WordPress](https://wordpress.org/support/article/debugging-in-wordpress/) for more information. (This message was added in version 5.0.0.)
I'm not sure if this is blocking or not (no pun intended), but we should probably gate the existence of the block to WP versions 5.6+ to avoid causing problems, so long as we technically support older versions.
After some more testing, I found I wasn't able to use the block until WP 5.8! These are old versions, and we're overdue to raise our minimum requirement here, but we should avoid loading the block on versions that will only cause notices and ultimately be non-functional.
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.
I've pushed the relevant change to require WP 5.8+ for the block to be registered, so if all else passes then this should be good to go.
Summary
Addresses issue:
Relevant technical choices
Uses inline styles as a stop-gap, because trying to load editor styles (or any styles) for this block when it's inside an
iFrame
using full-site editing does not work.PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist