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

Extensibility: Add publish panels support for plugins #6798

Merged
merged 2 commits into from
May 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CONTRIBUTORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,4 @@ This list is manually curated to include valuable contributions by volunteers th
| @Cloud887 |
| @hblackett |
| @vishalkakadiya |
| @c-shultz |
Copy link
Member

Choose a reason for hiding this comment

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

does this belong here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. See:

Continues work started by @c-shultz in #5795.

Copy link
Member

Choose a reason for hiding this comment

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

Perfect.

91 changes: 91 additions & 0 deletions edit-post/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ The [Dashicon](https://developer.wordpress.org/resource/dashicons/) icon slug st
- Required: No
- Default: _inherits from the plugin_


### `PluginPostStatusInfo`

Renders a row in the Status & Visibility panel of the Document sidebar.
Expand All @@ -127,3 +128,93 @@ const MyPluginPostStatusInfo = () => (
```


### `PluginPrePublishPanel`

Renders provided content to the pre-publish side panel in the publish flow (side panel that opens when a user first pushes "Publish" from the main editor).

_Example:_

```jsx
const { __ } = wp.i18n;
const { PluginPrePublishPanel } = wp.editPost;
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't these APIs just be EditorPrePublishPanel instead of Plugin...?

Copy link
Member Author

Choose a reason for hiding this comment

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

We prefixed all other occurrences with Plugin in the past, this just keeps the convention.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but I'm not sure it makes a lot of sense for every piece. :) PluginSidebar and the plugin menu item make sense because their entire purpose is plugin based.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not attached to this, we can update.
What about PluginPostStatustInfo, should we also rename to EditorPostStatusInfo?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did some more research. We already have PostPublishPanel. See:
https://github.com/WordPress/gutenberg/blob/master/editor/components/post-publish-panel/index.js#L66
which uses editor-post-publish-panel class name. It might be a bit confusing. Maybe we should keep this Plugin prefix to make it clear that this is meant to be used by plugins.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's fine, was just a comment because it felt a bit weird to go with Plugin as a generic prefix across the board.


const MyPluginPrePublishPanel = () => (
<PluginPrePublishPanel
className="my-plugin-pre-publish-panel"
title={ __( 'My panel title' ) }
initialOpen={ true }
>
{ __( 'My panel content' ) }
</PluginPrePublishPanel>
);
```

#### Props

##### className

An optional class name added to the panel.

- Type: `String`
- Required: No

##### title

Title displayed at the top of the panel.

- Type: `String`
- Required: No

##### initialOpen

Whether to have the panel initially opened. When no title is provided it is always opened.

- Type: `Boolean`
- Required: No
- Default: `false`


### `PluginPostPublishPanel`

Renders provided content to the post-publish panel in the publish flow (side panel that opens after a user publishes the post).

_Example:_

```jsx
const { __ } = wp.i18n;
const { PluginPostPublishPanel } = wp.editPost;

const MyPluginPostPublishPanel = () => (
<PluginPostPublishPanel
className="my-plugin-post-publish-panel"
title={ __( 'My panel title' ) }
initialOpen={ true }
>
{ __( 'My panel content' ) }
</PluginPostPublishPanel>
);
```

#### Props

##### className

An optional class name added to the panel.

- Type: `String`
- Required: No

##### title

Title displayed at the top of the panel.

- Type: `String`
- Required: No

##### initialOpen

Whether to have the panel initially opened. When no title is provided it is always opened.

- Type: `Boolean`
- Required: No
- Default: `false`
4 changes: 4 additions & 0 deletions edit-post/components/layout/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import EditorModeKeyboardShortcuts from '../keyboard-shortcuts';
import MetaBoxes from '../meta-boxes';
import { getMetaBoxContainer } from '../../utils/meta-boxes';
import Sidebar from '../sidebar';
import PluginPostPublishPanel from '../sidebar/plugin-post-publish-panel';
import PluginPrePublishPanel from '../sidebar/plugin-pre-publish-panel';

function Layout( {
mode,
Expand Down Expand Up @@ -84,6 +86,8 @@ function Layout( {
onClose={ closePublishSidebar }
forceIsDirty={ hasActiveMetaboxes }
forceIsSaving={ isSaving }
renderPrePublishExtension={ () => <PluginPrePublishPanel.Slot /> }
Copy link
Member

@jorgefilipecosta jorgefilipecosta May 17, 2018

Choose a reason for hiding this comment

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

Maybe here we can just have a renderPublishExtension prop, and leave it up to the component to query state and check if we are in pre-publish or post-publish and just render when we are in the desired state ( or both ).
Later we could even change for a more generic sidebar extension mechanism where we just extend all sidebars the component is responsible to query the state to discover if it should render something in a given state or not (e.g: if it is pre-publish) if it is the block sidebar etc...
This would even allow a plugin to extend the sidebars of other plugins :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Those two are mutually exclusive. You are either in pre-publish screen or post-publish screen. They are passed down to two different subcomponents. This is limited to publish sidebar only. Can you elaborate on your idea with some code examples? I must admit that I don't quite see how this part could relate to the plugin sidebar.

Copy link
Member

@jorgefilipecosta jorgefilipecosta May 22, 2018

Choose a reason for hiding this comment

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

Hi @gziolo here is a sample of the generic API for extending sidebars that I was referring: #6899.
Unfortunately, it ended up not being possible to use it in this case, because pre and post-publish state are not available globally for plugins to query. It uses local state.
But the initial idea was to render <SidebarExtender.Slot /> both pre and post-publish panels.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, this component has its own internal state which controls the logic for rendering individual panels. I will check your PR. Thanks for opening 👍

Copy link
Member

Choose a reason for hiding this comment

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

This will cause render reconciliation more than necessary because it's creating a new function each render.

Why not hoist to a top-level variable? Or could renderPrePublishExtension be interpreted as a component, so we could pass the PluginPrePublishPanel.Slot reference directly instead of a function?

Copy link
Member

Choose a reason for hiding this comment

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

Interesting, I would also think we would cause render reconciliation more than necessary because we are passing new props on each rerender, but the react samples for the Render Props pattern use something similar https://reactjs.org/docs/render-props.html, they are also creating inline functions and passing them as props. I see this practice being used widely. I'm curious if react has some an optimization for the inline functions rendering React components.

Copy link
Member Author

Choose a reason for hiding this comment

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

We probably can pass those slots as components in this case. Opening PR soon.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done: #6938.

renderPostPublishExtension={ () => <PluginPostPublishPanel.Slot /> }
/>
) }
<DocumentSidebar />
Expand Down
22 changes: 22 additions & 0 deletions edit-post/components/sidebar/plugin-post-publish-panel/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* WordPress dependencies
*/
import { createSlotFill, PanelBody } from '@wordpress/components';

const { Fill, Slot } = createSlotFill( 'PluginPostPublishPanel' );

const PluginPostPublishPanel = ( { children, className, title, initialOpen = false } ) => (
<Fill>
<PanelBody
className={ className }
initialOpen={ initialOpen || ! title }
title={ title }
>
{ children }
</PanelBody>
</Fill>
);

PluginPostPublishPanel.Slot = Slot;

export default PluginPostPublishPanel;
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`PluginPostPublishPanel renders fill properly 1`] = `
<div>
<PanelBody
className="my-plugin-post-publish-panel"
initialOpen={true}
key="1---0/.0"
title="My panel title"
>
<div
className="components-panel__body my-plugin-post-publish-panel is-opened"
>
<h2
className="components-panel__body-title"
>
<Button
aria-expanded={true}
className="components-panel__body-toggle"
onClick={[Function]}
>
<button
aria-expanded={true}
className="components-button components-panel__body-toggle"
onClick={[Function]}
type="button"
>
<Dashicon
icon="arrow-up"
>
<svg
aria-hidden={true}
className="dashicon dashicons-arrow-up"
focusable="false"
height={20}
role="img"
viewBox="0 0 20 20"
width={20}
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M7 13l4.03-6L15 13H7z"
/>
</svg>
</Dashicon>
My panel title
</button>
</Button>
</h2>
My panel content
</div>
</PanelBody>
</div>
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* External dependencies
*/
import { mount } from 'enzyme';

/**
* WordPress dependencies
*/
import { SlotFillProvider } from '@wordpress/components';

/**
* Internal dependencies
*/
import PluginPostPublishPanel from '../';

describe( 'PluginPostPublishPanel', () => {
test( 'renders fill properly', () => {
const wrapper = mount(
<SlotFillProvider>
<PluginPostPublishPanel
className="my-plugin-post-publish-panel"
title="My panel title"
initialOpen={ true }
>
My panel content
</PluginPostPublishPanel>
<PluginPostPublishPanel.Slot />
</SlotFillProvider>
);

expect( wrapper.find( 'Slot' ).children() ).toMatchSnapshot();
} );
} );
22 changes: 22 additions & 0 deletions edit-post/components/sidebar/plugin-pre-publish-panel/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* WordPress dependencies
*/
import { createSlotFill, PanelBody } from '@wordpress/components';

const { Fill, Slot } = createSlotFill( 'PluginPrePublishPanel' );

const PluginPrePublishPanel = ( { children, className, title, initialOpen = false } ) => (
<Fill>
<PanelBody
className={ className }
initialOpen={ initialOpen || ! title }
title={ title }
>
{ children }
</PanelBody>
</Fill>
);

PluginPrePublishPanel.Slot = Slot;

export default PluginPrePublishPanel;
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`PluginPrePublishPanel renders fill properly 1`] = `
<div>
<PanelBody
className="my-plugin-pre-publish-panel"
initialOpen={true}
key="1---0/.0"
title="My panel title"
>
<div
className="components-panel__body my-plugin-pre-publish-panel is-opened"
>
<h2
className="components-panel__body-title"
>
<Button
aria-expanded={true}
className="components-panel__body-toggle"
onClick={[Function]}
>
<button
aria-expanded={true}
className="components-button components-panel__body-toggle"
onClick={[Function]}
type="button"
>
<Dashicon
icon="arrow-up"
>
<svg
aria-hidden={true}
className="dashicon dashicons-arrow-up"
focusable="false"
height={20}
role="img"
viewBox="0 0 20 20"
width={20}
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M7 13l4.03-6L15 13H7z"
/>
</svg>
</Dashicon>
My panel title
</button>
</Button>
</h2>
My panel content
</div>
</PanelBody>
</div>
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* External dependencies
*/
import { mount } from 'enzyme';

/**
* WordPress dependencies
*/
import { SlotFillProvider } from '@wordpress/components';

/**
* Internal dependencies
*/
import PluginPrePublishPanel from '../';

describe( 'PluginPrePublishPanel', () => {
test( 'renders fill properly', () => {
const wrapper = mount(
<SlotFillProvider>
<PluginPrePublishPanel
className="my-plugin-pre-publish-panel"
title="My panel title"
initialOpen={ true }
>
My panel content
</PluginPrePublishPanel>
<PluginPrePublishPanel.Slot />
</SlotFillProvider>
);

expect( wrapper.find( 'Slot' ).children() ).toMatchSnapshot();
} );
} );
2 changes: 2 additions & 0 deletions edit-post/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ export function initializeEditor( id, post, settings ) {
};
}

export { default as PluginPostPublishPanel } from './components/sidebar/plugin-post-publish-panel';
export { default as PluginPostStatusInfo } from './components/sidebar/plugin-post-status-info';
export { default as PluginPrePublishPanel } from './components/sidebar/plugin-pre-publish-panel';
export { default as PluginSidebar } from './components/sidebar/plugin-sidebar';
export { default as PluginSidebarMoreMenuItem } from './components/header/plugin-sidebar-more-menu-item';
14 changes: 11 additions & 3 deletions editor/components/post-publish-panel/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class PostPublishPanel extends Component {
}

render() {
const { isScheduled, onClose, forceIsDirty, forceIsSaving } = this.props;
const { isScheduled, onClose, forceIsDirty, forceIsSaving, renderPrePublishExtension, renderPostPublishExtension } = this.props;
const { loading, submitted } = this.state;
return (
<div className="editor-post-publish-panel">
Expand All @@ -82,9 +82,17 @@ class PostPublishPanel extends Component {
/>
</div>
<div className="editor-post-publish-panel__content">
{ ! loading && ! submitted && <PostPublishPanelPrepublish /> }
{ ! loading && ! submitted && (
<PostPublishPanelPrepublish>
{ renderPrePublishExtension() }
</PostPublishPanelPrepublish>
) }
{ loading && ! submitted && <Spinner /> }
{ submitted && <PostPublishPanelPostpublish /> }
{ submitted && (
<PostPublishPanelPostpublish>
{ renderPostPublishExtension() }
</PostPublishPanelPostpublish>
) }
</div>
</div>
);
Expand Down
Loading