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.

Sign up for GitHub

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

Jump to bottom

Refactor the initialization of the editor, require only a post ID #6384

Merged
merged 18 commits into from
Jun 6, 2018
Merged

Refactor the initialization of the editor, require only a post ID #6384

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
7f0177a
Refactor the initialization of the editor, require only a post ID ins…
youknowriad Apr 24, 2018
059cde3
Fix phpcs tests
youknowriad Apr 25, 2018
28be8f0
Load dynamically entities by kind
youknowriad May 3, 2018
d37783d
Fix selector hasEntitiesByKind
youknowriad May 8, 2018
aef4ada
Remove useless posts reducer
youknowriad May 8, 2018
b69f5c1
Move LoadKindEntities to the entities file
youknowriad May 8, 2018
5b1076e
Add unit tests
youknowriad May 10, 2018
f2d4a87
Fix unit tests
youknowriad May 10, 2018
7127cc0
Fix custom post types preloading
youknowriad May 11, 2018
2eb7f16
Avoid generating a new reducer on each call if the config didn't change
youknowriad May 17, 2018
d29c910
Rename `defaultPost => `overridePost` to clarify its meaning
youknowriad May 17, 2018
66b44bd
Fix unit tests
youknowriad May 17, 2018
8a067ed
Fix unit tests after rebase
youknowriad May 28, 2018
91f36ab
Adding await when calling async generators
youknowriad May 28, 2018
48425b2
Switch initializeEditor arguments
youknowriad May 28, 2018
5f2b411
Fix unit tests
youknowriad Jun 5, 2018
e743830
Returning the _links from the preloaded endpoints
youknowriad Jun 5, 2018
ab1e150
Code style fixes
youknowriad Jun 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Rename defaultPost => overridePost` to clarify its meaning
  • Loading branch information
@youknowriad
youknowriad committed Jun 5, 2018
commit d29c9102e9ab55011855b556264ac8d3e50c1377
4 changes: 2 additions & 2 deletions edit-post/editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { EditorProvider, ErrorBoundary } from '@wordpress/editor';
*/
import Layout from './components/layout';

function Editor( { settings, hasFixedToolbar, post, defaultPost, onError, ...props } ) {
function Editor( { settings, hasFixedToolbar, post, overridePost, onError, ...props } ) {
if ( ! post ) {
return null;
}
Expand All @@ -20,7 +20,7 @@ function Editor( { settings, hasFixedToolbar, post, defaultPost, onError, ...pro
};

return (
<EditorProvider settings={ editorSettings } post={ { ...post, ...defaultPost } } { ...props }>
<EditorProvider settings={ editorSettings } post={ { ...post, ...overridePost } } { ...props }>
<ErrorBoundary onError={ onError }>
<Layout />
</ErrorBoundary>
Expand Down
24 changes: 12 additions & 12 deletions edit-post/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,18 @@ window.jQuery( document ).on( 'heartbeat-tick', ( event, response ) => {
* an unhandled error occurs, replacing previously mounted editor element using
* an initial state from prior to the crash.
*
* @param {Object} postId ID of the post to edit.
* @param {Object} postType Post type of the post to edit.
* @param {Element} target DOM node in which editor is rendered.
* @param {?Object} settings Editor settings object.
* @param {Object} defaultPost Post initilization object
* @param {Object} postId ID of the post to edit.
* @param {Object} postType Post type of the post to edit.
* @param {Element} target DOM node in which editor is rendered.
* @param {?Object} settings Editor settings object.
* @param {Object} overridePost Post properties to override.
*/
export function reinitializeEditor( postId, postType, target, settings, defaultPost ) {
export function reinitializeEditor( postId, postType, target, settings, overridePost ) {
unmountComponentAtNode( target );
const reboot = reinitializeEditor.bind( null, postId, target, settings, defaultPost );
const reboot = reinitializeEditor.bind( null, postId, target, settings, overridePost );

render(
<Editor settings={ settings } onError={ reboot } postId={ postId } postType={ postType } defaultPost={ defaultPost } recovery />,
<Editor settings={ settings } onError={ reboot } postId={ postId } postType={ postType } overridePost={ overridePost } recovery />,
target
);
}
Expand All @@ -54,11 +54,11 @@ export function reinitializeEditor( postId, postType, target, settings, defaultP
* @param {Object} postId ID of the post to edit.
* @param {Object} postType Post type of the post to edit.
* @param {?Object} settings Editor settings object.
* @param {Object} defaultPost Post initilization object
* @param {Object} overridePost Post properties to override.
*
* @return {Object} Editor interface.
*/
export function initializeEditor( id, postId, postType, settings, defaultPost ) {
export function initializeEditor( id, postId, postType, settings, overridePost ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a potential future where postId becomes optional, would we want to support calling this as simply...

initializeEditor( 'gutenberg', 'post' );

i.e. with postType before the optional postId

Asides:

  • id is pretty useless now. It was originally intended for separate instances of an editor, which we don't really support. Maybe it should just be dropped.
  • At what point might we consider an object argument? Since we could have many variations (post with ID and no overrides, post with no ID and overrides, post with overrides but no settings)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll keep the id for now. I'm not certain if we'll support multi-instances at some point but prefer to keep it for now.

An object argument: I don't know 🤷‍♂️, It may feel like a component though and maybe keeping it as clarify the distinction?

if ( 'production' !== process.env.NODE_ENV ) {
// Remove with 3.0 release.
window.console.info(
Expand All @@ -69,12 +69,12 @@ export function initializeEditor( id, postId, postType, settings, defaultPost )
}

const target = document.getElementById( id );
const reboot = reinitializeEditor.bind( null, postId, postType, target, settings, defaultPost );
const reboot = reinitializeEditor.bind( null, postId, postType, target, settings, overridePost );

registerCoreBlocks();

render(
<Editor settings={ settings } onError={ reboot } postId={ postId } postType={ postType } defaultPost={ defaultPost } />,
<Editor settings={ settings } onError={ reboot } postId={ postId } postType={ postType } overridePost={ overridePost } />,
target
);

Expand Down