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.

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

Editor: Expose editor components as reusable pieces #2761

Closed
aduth opened this issue Sep 21, 2017 · 11 comments · Fixed by #3608
Closed

Editor: Expose editor components as reusable pieces #2761

aduth opened this issue Sep 21, 2017 · 11 comments · Fixed by #3608
Assignees
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript General Interface Parts of the UI which don't fall neatly under other labels.

Comments

@aduth
Copy link
Member

aduth commented Sep 21, 2017

Previously: #2065

The current implementation of the editor is very specific to the post editor screen, but we will want to support cases where parts of the editor can be used in different contexts. A component like <VisualEditorBlockList />, for example, would be a very useful substitute for how someone might currently otherwise use a single TinyMCE instance on a page. To support this, we would still need block values to be coordinated through editor state, ideally in a manner less specific to managing a post entity.

Proposal:

Expose a number of Editor components, the root of which an <Editor> component serves as the interface through which changes are surfaced and state is initialized:

<Editor>
	<header>
		<Editor.UndoRedo />
	</header>
	<Editor.BlockList />
	<Editor.InserterButton />
	<aside>
		<Editor.BlockInspector />
	</aside>
</Editor>

How changes are surfaced should be guided by use-cases. It may be that Editor accepts a single onChange prop which is invoked with the string of content, but it could be that a consumer would need more raw access to actions as they are dispatched. Further, frequent serializations will likely have a negative impact on performance.

One option is for the consumer to initialize the Redux store themselves, thus having access to applying their own middlewares or store enhancers to gain access to raw changes as they occur.

const store = createEditorStore( /* ...enhancers */ );

const editor = <Editor store={ store } />;

// ...later

const content = getContent( store.getState() );

In this model, a post editor would separately manage post properties such as visibility, author, terms, etc, and the underlying <Editor /> would hold responsibility for maintaining the value of a post's content.

This would also bring alignment between blocks and editor, and I suggest we eliminate the distinction. It could be that the Editor component receives a configuration of blocks, a subset of all registered options (#2065), or that blocks are registered to the editor instance itself. In either case, I see block registration being specific to the idea of an editor.

const editor = createEditorInstance();

editor.registerBlock( /* ... */ );

I think it's important that we don't become excessively granular by artificially separating concepts. Where we currently distinguish editor from blocks, we should instead distinguish editor (and blocks) from post editing.

- /blocks
/editor
+ /post-editor
@aduth aduth added General Interface Parts of the UI which don't fall neatly under other labels. Framework Issues related to broader framework topics, especially as it relates to javascript labels Sep 21, 2017
@youknowriad
Copy link
Contributor

While I may disagree on some of the proposed solutions (the way we initilialize the state and the blocks config), I'm totally on board with the global idea here.

Further, frequent serializations will likely have a negative impact on performance.

not if the value is a block list instead of a string and I think it makes more sense since the blocks output can be spread into multiple places (post content, post meta, site options).


Also, I see us providing some built-in variations:

  • PostEditor similar to the current editor
  • LightEditor (or P2Editor) Not sure we should provide this built-in, but could serve as a validation for this approach.
  • TemplateEditor (comes later for Customization)

I think we should try to break this into smaller steps:

  • Merge blocks and editor modules
  • Extract the reusable components (Editor.UndoRedo, Editor.Inserter, Editor.FullInserter, Editor.BlockInspector, Editor.BlockList, Editor.MultiselectionHeader)
  • Data Refactoring: I'm not sure about the details here, but we should separate the current store into maybe two stores (or leverate local state in some places). The Editor Store should only deal with blocks. (this may not be so clear right now, but I think it's something that will eventually become "obvious" later)
  • Make the available block list configurable per editor
  • Build Alaternative layouts (PostEditor, LightEditor)

@aduth
Copy link
Member Author

aduth commented Sep 21, 2017

not if the value is a block list instead of a string and I think it makes more sense since the blocks output can be spread into multiple places (post content, post meta, site options).

This is still possible if left to the consumer to initialize the block store; just more low-level.

const store = createEditorStore( /* ...enhancers */ );

const editor = <Editor store={ store } />;

// ...later

const blocks = getBlocks( store.getState() ); // <-- Do with blocks list as you please

The thought with initializing the store and allowing enhancers to be passed is that it enables the consumer to introduce middlewares or effects to hook into editing actions as they occur. We'd want to develop some use-cases here; perhaps triggering an analytics event when a block is inserted.

Or we could support both cases, where a default state is initialized if not explicitly passed, still allowing for a simpler onChange callback.

@aduth
Copy link
Member Author

aduth commented Sep 22, 2017

@youknowriad Do you have any particular use-cases in mind where onChange would be be a particularly good fit instead of retrieving blocks by direct store reference at the points in time content retrieval is needed?

Do you think it would be an easier path to rename the current editor folder as post-editor and blocks as editor, then pull generic editor pieces from post-editor into the new editor folder?

@youknowriad
Copy link
Contributor

Do you have any particular use-cases in mind where onChange would be a particularly good fit instead of retrieving blocks by direct store reference at the points in time content retrieval is needed?

Not a use-case really, but just a "conception" preference, I like to see the Editor as just an input for blocks. and having a parent component to deal with saving/publishing ... (and all side effects), but I know this is very different from what we're doing currently and could be done progressively, maybe as part of 3rd step I mention above (if you all agree).

Do you think it would be an easier path to rename the current editor folder as post-editor and blocks as editor, then pull generic editor pieces from post-editor into the new editor folder?

That's exactly what I was trying to achieve with my previous PRs. and blocks and editor could arguably be merged together (as generic pieces to build the different editors). It seemed to me that everyone was not on board with this idea, so maybe a more "pragmatic" approach is to merge everything into a single module to start with, build reusable pieces inside this module and extract them at the end of the refactoring.

@mtias
Copy link
Member

mtias commented Sep 22, 2017

Not a use-case really, but just a "conception" preference, I like to see the Editor as just an input for blocks.

We need to clarify what we mean with "Editor". In the example above, it is the container of all editing functionality, while Editor.BlockList is the one that specifically handles blocks.

@youknowriad
Copy link
Contributor

youknowriad commented Oct 20, 2017

I looked into this a bit, and as a next step, I propose to change the directory structure of the current editor module like so:

editor/components

Would contain all the reusable components to build an editor exported in wp.editor:

  • The state container: EditorProvider (could be renamed just Editor),

  • Generic Block Editor Components BlockInspector, Inserter (simple and inserter with quick buttons)

  • Post Editor reusable components PostTitle, PublishButton, PublishButtonWithDropdown, PostSchedule, PostVisibility, SavedState, DocumentOutline, PostPermalink, Metaboxes

    • All these must be refactored slightly to drop all the "layout" pieces and keep only the "forms". For example, the sidebar panels like PostTaxonomies, we should extract only the PostTaxonomyForms into a reusable component without the expandable panel behavior that is specific to our current UI. (This step could be done in a separate PR preceding the directory structure change)
  • BlockList Or all what's being rendered currently under the VisualEditorBlockList component which includes: BlockToolbar, BlockMover, BlockSwitcher, Block... All these should be under a parent directory BlockList in the components directory.

editor/post-editor

This is where composing the reusable bits happen: This would include:

  • A Header to compose the reusable header pieces
  • A Sidebar with the tabs switching
  • The sidebar panels by reusing the extracted components
    and a global PostEditor component

other layouts

Alternative layouts can reuse the different components exported in wp.editor to build their alternative to the editor/post-editor UI. (Say P2Editor)

Let's discuss this before I start working on it.

@aduth
Copy link
Member Author

aduth commented Oct 20, 2017

For example, the sidebar panels like PostTaxonomies, we should extract only the PostTaxonomyForms into a reusable component without the expandable panel behavior that is specific to our current UI.

Would the reusable bit here be the HierarchicalTermSelector and FlatTermSelector components? Maybe PostTaxonomies as we have it is the specific post-editor UI implementation.

@youknowriad
Copy link
Contributor

Would the reusable bit here be the HierarchicalTermSelector and FlatTermSelector components?

Yes, and maybe add a wrapper to these components to map and display them for all custom taxonomies. The idea is to get rid of the expandable panel because it's post-editor specific.

Also, the Taxonomies was just an example, it's the same thing for all the sidebar panels.

@aduth
Copy link
Member Author

aduth commented Oct 20, 2017

Also, the Taxonomies was just an example, it's the same thing for all the sidebar panels.

Right, just getting at that it might be fine to have our own UI wrappers using panels if there are already other lower-level primitives for another editor interface to be using. We might not need the component which maps on all taxonomies, even (maybe, I could see this going either way).

@mtias
Copy link
Member

mtias commented Nov 20, 2017

Do we want to keep this open further? @aduth @youknowriad

@youknowriad
Copy link
Contributor

Let's keep it open, I plan to close it soon once we land the two last PRs above and then move all the remaining components to a subfolder (for now) post-editor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript General Interface Parts of the UI which don't fall neatly under other labels.
Projects
None yet
3 participants