-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Make calendar block resilient against editor module not being present #16161
Make calendar block resilient against editor module not being present #16161
Conversation
@@ -61,9 +61,13 @@ class CalendarEdit extends Component { | |||
} | |||
|
|||
export default withSelect( ( select ) => { | |||
const coreEditorSelect = select( 'core/editor' ); |
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 is very concerning as it might break many 3rd party blocks when they are inserted. Do you have any issue where you track discussion related to how to ensure that blocks are rendered in the proper context?
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.
What if other block explicitly imports core/editor package making this store available?
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.
What if other block explicitly imports core/editor package making this store available?
It may not break the block, but might produce weird results. For instance, there's no post object or something like that. This is one of those things we need to document properly before the release. It's still on the table that the blocks should opt-in or opt-out of the widgets screen. I think we need more wide usage of the widgets screen here before making an informed decision.
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.
Yes, some blocks that rely on the editor module may not work on the widget screen, because we don't have a post there. There is not exactly breaking back-compatibility because these blocks were created for the post editor and they still work on the post editor; I guess an option to disable/enable blocks from the widget screen may be a possibility.
I added the "Needs dev note" label, that way we don't forget documenting this before the WP release including these changes. |
…rnmobile/open-video-by-browser-for-android * 'master' of https://github.com/WordPress/gutenberg: (34 commits) [RNMobile] Native mobile release v1.7.0 (#16156) Scripts: Document and simplify changing file entry and output of build scripts (#15982) Block library: Refactor Heading block to use class names for text align (#16035) Make calendar block resilient against editor module not being present (#16161) Bump plugin version to 5.9.1 Editor: Fix the issue where statics for deprecated components were not hoisted (#16152) Build Tooling: Use "full" `npm install` for Build Artifacts Travis task (#16166) Scripts: Fix naming conventions for function containing CLI keyword (#16091) Add native support for Inserter (Ported from gutenberg-mobile) (#16114) docs(components/higher-order/with-spoken-messages): fix issue in example code (#16144) docs(components/with-focus-return): fix typo in README.md (#16143) docs(block-editor/components/inspector-controls): fix image path in README.md (#16145) Add mention of on Figma to CONTRIBUTING.md (#16140) Bring greater consistency to placeholder text for media blocks. (#16135) Add descriptive text and a link to embed documentation in embed blocks (#16101) Update toolbar-text.png (#16102) Updating changelogs for the Gutenberg 5.9 packages release chore(release): publish [RNMobile] Fix pasting text on Post Title (#16116) Bump plugin version to 5.9.0 ... # Conflicts: # packages/block-library/src/video/video-player.android.js
Since the widgets screen is not released yet, let's delay this dev note :) |
Description
The block should not assume the editor store is available.
How has this been tested?
I added a calendar block to one of the widget areas.
I switched to this branch and cherry-picked #16160.
I verified the calendar block still works. I go to branch #16160 without these changes the block crashes.