Initialize the state before rendering widgets editor #25736
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
adamziel
added
[Feature] Widgets Screen
|
Size Change: +170 B (0%) Total Size: 1.18 MB
ℹ️ View Unchanged
|
noisysocks
reviewed
Oct 1, 2020
| }, [] ); | ||
|
|
||
| if ( ! stateInitialized ) { | ||
| return <div></div>; |
There was a problem hiding this comment.
Could this be return null;? Having an empty div in the DOM structure isn't ideal.
| yield persistStubPost( buildWidgetAreasPostId(), [] ); | ||
| } | ||
|
|
||
| export const persistStubPost = function* ( id, blocks ) { |
There was a problem hiding this comment.
This is exported so needs a doc comment.
| export const persistStubPost = function* ( id, blocks ) { | ||
| const stubPost = createStubPost( id, blocks ); | ||
| const args = [ KIND, POST_TYPE, id ]; | ||
| yield dispatch( 'core', 'startResolution', 'getEntityRecord', args ); |
There was a problem hiding this comment.
I know from experience that this is the magic that prevents core-data from trying to resolve the entity, but I imagine most would not pick that up. Let's add a doc comment.
| const { initializeState } = useDispatch( 'core/edit-widgets' ); | ||
| const [ stateInitialized, setStateInitialized ] = useState( false ); | ||
| useEffect( () => { | ||
| initializeState(); |
There was a problem hiding this comment.
It's strange to see an action that looks so... imperative. I wonder if instead of an action we should use a resolver called getStubPost or getWidgetsEditorPost and then discard the return value? Third party plugins might find the return value useful.
Not a strong opinion, just an idea, take it with a grain of salt, etc.
There was a problem hiding this comment.
Oh. There's already getWidgetAreas which is called here:
If we have that and are calling it when WidgetAreasBlockEditorProvider mounts, why is this PR necessary?
There was a problem hiding this comment.
@noisysocks we need this PR to explicitly do things in certain order and avoid race conditions. Without this PR the post is requested before the widgets resolver initialize it. We could maybe line up the code so that it doesn’t happen, but I’d rather do it explicitly.
kevin940726
approved these changes
Oct 5, 2020
There was a problem hiding this comment.
Mostly looks good to me. Just same as what @noisysocks already mentioned 👍
adamziel
added a commit
that referenced
this pull request
Oct 7, 2020
* Initialize the state before rendering widgets editor * Replace empty div with null * Document persistStubPost * Document persistStubPost further
Merged
draganescu
pushed a commit
that referenced
this pull request
Oct 7, 2020
* Include edit-widgets php files in build (#25792) * Fix PHP warining in widget utils controller (#25797) * Fix PHP warning in WP_REST_Widget_Utils_Controller The `WP_REST_Widget_Utils_Controller::is_valid_widget` method needs to be public in order to be accessible as a callback (since it's being called from outside the class, via `call_user_func`). This commit fixes the warning by changing the method's visibility from `private` to `public`. * Ammend the inline documentation. Add `* @access public` * [Widgets Editor] Fix insertion point in widget areas (#25727) * Move hooks inside the newly created Interface component * Fix insertion at the top of the widget area * Fix widget insertion from sidebar block library by using a declared insertionPoint prop (#25763) * Fix widget insertion from sidebar block library by using a declared insertionPoint prop * Add comments * Optimize by avoiding `getEntityRecord` call unless needed Co-authored-by: Daniel Richards <daniel.richards@automattic.com> * Initialize the state before rendering widgets editor (#25736) * Initialize the state before rendering widgets editor * Replace empty div with null * Document persistStubPost * Document persistStubPost further * Bump version to 9.1.1 * Update changelog * Fix spaces in changelog.txt * Adjust spaces in changelog.txt * Fix link formatting in the changelog Co-authored-by: Jon Surrell <jon.surrell@automattic.com> Co-authored-by: David Biňovec <david.binovec@gmail.com> Co-authored-by: Kai Hao <kevin830726@gmail.com> Co-authored-by: Daniel Richards <daniel.richards@automattic.com>
This was referenced Oct 7, 2020
Merged
1 task
1 task
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
The widgets editor uses a "stub post" as a container for all widget areas. It must be initialized upfront or the data layer would attempt an API resolution. This PR adds that initialization.
How has this been tested?
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: