Legacy Widget: Improve backwards compatibility #30709
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
noisysocks
added
[Type] Bug
|
Size Change: +31.3 kB (+2%) Total Size: 1.46 MB
ℹ️ View Unchanged
|
kevin940726
reviewed
Apr 14, 2021
- Fire 'widget-added' and 'widget-updated' events when the widget form is initialized or when it changes. This is what most widgets' scripts use to initialize and/or attach event listeners. - Include hidden inputs that most widgets expect to exist in the DOM.
Co-authored-by: Kai Hao <kevin830726@gmail.com>
Co-authored-by: Kai Hao <kevin830726@gmail.com>
noisysocks
force-pushed
the
fix/legacy-widget-events
branch
from
3bea1d7 to
6196284
Compare
|
Ready for re-review. |
kevin940726
reviewed
Apr 14, 2021
packages/block-library/package.json
Outdated
| @@ -58,6 +58,7 @@ | |||
| "@wordpress/viewport": "file:../viewport", | |||
| "classnames": "^2.2.5", | |||
| "fast-average-color": "4.3.0", | |||
| "jquery": "^3.5.1", | |||
There was a problem hiding this comment.
Off topic: I always think the version here is misleading, I think it even came up once during my coding test interview 😅. Maybe we should just pin it as * to make it more clear that it's not the version we are using at all.
However, that would become really weird for direct consumer of the npm package itself. Speaking of that, does it mean we're requiring jquery for this package now? I wonder what we could do about it 🤔. Maybe a fallback as you mentioned?
There was a problem hiding this comment.
I... do not know 😅
@gziolo: Is this OK? Or is it better to access jQuery via the window global? We need to use it for backwards compatibility.
There was a problem hiding this comment.
I think we did a lot of work in the past to avoid jQuery as a dependency in our packages, So I think since this is specifically for BC, we should use the global (if present).
Makes me wonder whether the legacy widget block should be in the block library package or in edit-widgets
There was a problem hiding this comment.
That's a good question to ask whether the Legacy Widget block would be useful outside of the widgets screen and the Customizer? I wouldn't mind it was moved to the edit-widgets package. It's already the case for the Widget Area block:
https://github.com/WordPress/gutenberg/tree/trunk/packages/edit-widgets/src/blocks/widget-area
There was a problem hiding this comment.
I don't think the legacy widget block should be available anywhere else but the block editor that edits widget areas. The legacy widget block is tied to the way widgets are stored and it would be a bad idea to bring this all over the place.
There was a problem hiding this comment.
Makes me wonder whether the legacy widget block should be in the block library package or in edit-widgets
It was in edit-widgets but I moved it back to block-library in #29960 for two reasons:
- It means that
customize-widgetscan use the block. - It ensures that we are not relying on anything in
edit-widgetsto implement the block. I think this is important as it gives us the option to, in the future, allow Legacy Widget to be used in the site editor or wherever else we may want to allow it. I could see a case for block-based themes wanting to use a widget when there is no block equivalent.
The legacy widget block is tied to the way widgets are stored
This is not true as of #29960. Care has been taken to make Legacy Widget work like any other block that relies on the REST API: Latest Posts, Latest Comments, etc.
|
Not sure if it's related, but I'm seeing a scrollbar in the preview of the text legacy widget, and it cannot be scrolled.
(Using theme Twenty Twenty) |
I haven't this behaviour on my side |
gziolo
reviewed
Apr 14, 2021
Looks like the styling of the widgets editor in Twenty Twenty is all kinds of messed up. Let's fix it separately. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds back two things that I removed in #29960 but shouldn't have removed 🙂
How has this been tested?
gutenberg.php) so that you can test the Legacy Widget block with all core widgets.Screenshots
Before:
After:
Types of changes
Bug fix
Checklist:
*.native.jsfiles for terms that need renaming or removal).