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

Edit Post: Refactor state handling for the sidebar #5435

Merged
merged 4 commits into from
Mar 9, 2018
Merged

Edit Post: Refactor state handling for the sidebar #5435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
06493ed
Edit Post: Remove redundant activate sidebar action
gziolo Mar 6, 2018
bc8cced
Edit Post: Simplify sidebar handing in the store
gziolo Mar 6, 2018
6f95ede
Edit Post: Improve Show Advanced Settings button
gziolo Mar 7, 2018
4c9d517
Edit Post: Refactor modified components to use new data module
gziolo Mar 7, 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
10 changes: 6 additions & 4 deletions edit-post/api/sidebar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
/* External dependencies */
import { isFunction } from 'lodash';

/**
* WordPress dependencies
*/
import { dispatch } from '@wordpress/data';

/* Internal dependencies */
import store from '../store';
import { setGeneralSidebarActivePanel, openGeneralSidebar } from '../store/actions';
import { applyFilters } from '@wordpress/hooks';

const sidebars = {};
Expand Down Expand Up @@ -93,6 +96,5 @@ export function getSidebarSettings( name ) {
* @return {void}
*/
export function activateSidebar( name ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@omarreiss @xyfi @IreneStr, can we get rid of this function completely and start using dispatch( 'edit-post' ).openGeneralSidebar( name ); directly?

Copy link
Member Author

@gziolo gziolo Mar 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Question 2, should we rename openGeneralSidebar to openEditorSidebar and use the name editor sidebar in other places?

On the other hand, I created isEditorSidebarOpened selector to check if block or document settings are opened in the sidebar.

Copy link
Member

Choose a reason for hiding this comment

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

I guess it would be fine to get rid of this function. I still see the use for a higher level API like this because it simply requires less knowledge of the user. But maybe it is a bit premature to be adding API's like this one at this stage. I would like to make a knowledge matrix further down the road to map what knowledge is needed to solve a particular kind of problem in Gutenberg as a plugin author. Based on that we could always decide to reintroduce helpers like this one.

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 will open a separate PR for that together with docs update. This PR got big once I started integrating data module. We can wrap it later back with something that you will find out helpful when integrating plugins.

store.dispatch( openGeneralSidebar( 'plugin' ) );
store.dispatch( setGeneralSidebarActivePanel( 'plugin', name ) );
dispatch( 'core/edit-post' ).openGeneralSidebar( name );
}
64 changes: 24 additions & 40 deletions edit-post/components/header/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import { connect } from 'react-redux';

/**
* WordPress dependencies
*/
Expand All @@ -13,35 +8,26 @@ import {
PostSavedState,
PostPublishPanelToggle,
} from '@wordpress/editor';
import { withDispatch, withSelect } from '@wordpress/data';
import { compose } from '@wordpress/element';

/**
* Internal dependencies
*/
import './style.scss';
import MoreMenu from './more-menu';
import HeaderToolbar from './header-toolbar';
import {
getOpenedGeneralSidebar,
isPublishSidebarOpened,
hasMetaBoxes,
isSavingMetaBoxes,
} from '../../store/selectors';
import {

function Header( {
isEditorSidebarOpened,
openGeneralSidebar,
closeGeneralSidebar,
isPublishSidebarOpened,
togglePublishSidebar,
} from '../../store/actions';

function Header( {
isGeneralSidebarEditorOpen,
onOpenGeneralSidebar,
onCloseGeneralSidebar,
isPublishSidebarOpen,
onTogglePublishSidebar,
hasActiveMetaboxes,
isSaving,
} ) {
const toggleGeneralSidebar = isGeneralSidebarEditorOpen ? onCloseGeneralSidebar : onOpenGeneralSidebar;
const toggleGeneralSidebar = isEditorSidebarOpened ? closeGeneralSidebar : openGeneralSidebar;

return (
<div
Expand All @@ -51,25 +37,25 @@ function Header( {
tabIndex="-1"
>
<HeaderToolbar />
{ ! isPublishSidebarOpen && (
{ ! isPublishSidebarOpened && (
<div className="edit-post-header__settings">
<PostSavedState
forceIsDirty={ hasActiveMetaboxes }
forceIsSaving={ isSaving }
/>
<PostPreviewButton />
<PostPublishPanelToggle
isOpen={ isPublishSidebarOpen }
onToggle={ onTogglePublishSidebar }
isOpen={ isPublishSidebarOpened }
onToggle={ togglePublishSidebar }
forceIsDirty={ hasActiveMetaboxes }
forceIsSaving={ isSaving }
/>
<IconButton
icon="admin-generic"
onClick={ toggleGeneralSidebar }
isToggled={ isGeneralSidebarEditorOpen }
isToggled={ isEditorSidebarOpened }
label={ __( 'Settings' ) }
aria-expanded={ isGeneralSidebarEditorOpen }
aria-expanded={ isEditorSidebarOpened }
/>
<MoreMenu key="more-menu" />
</div>
Expand All @@ -78,18 +64,16 @@ function Header( {
);
}

export default connect(
( state ) => ( {
isGeneralSidebarEditorOpen: getOpenedGeneralSidebar( state ) === 'editor',
isPublishSidebarOpen: isPublishSidebarOpened( state ),
hasActiveMetaboxes: hasMetaBoxes( state ),
isSaving: isSavingMetaBoxes( state ),
} ),
{
onOpenGeneralSidebar: () => openGeneralSidebar( 'editor' ),
onCloseGeneralSidebar: closeGeneralSidebar,
onTogglePublishSidebar: togglePublishSidebar,
},
undefined,
{ storeKey: 'edit-post' }
export default compose(
withSelect( ( select ) => ( {
isEditorSidebarOpened: select( 'core/edit-post' ).isEditorSidebarOpened(),
Copy link
Member

Choose a reason for hiding this comment

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

Not directly related to this pr, and even if we decide to some change it should not be in this PR. We are always repeating select( 'core/edit-post' ). This makes me wonder if we should not do something like: const editPostSelect = select( 'core/edit-post' ). Or add an argument to withSelect like withSelect( 'core/edit-post', ( select )...

Copy link
Member

Choose a reason for hiding this comment

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

FWIW this is how I'd documented examples in the updated @wordpress/data documentation:

https://github.com/WordPress/gutenberg/tree/master/data#withselect-mapselecttoprops-function--function

While it means you can't do an implicit arrow return, I think the whole thing ends up being more readable overall (implicit arrow returns for objects are always questionable).

Copy link
Member Author

@gziolo gziolo Mar 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, it has been discussed a few times in the past. I will find some link and share on Monday :) I might even share a similar idea you mentioned: withSelect( 'core/edit-post', ( select ). I think I saw @aduth using the following:

withSelect( ( select) => {
    const { mySelector } = select( 'core/edit-post' );
    return {
        // some code here
    };
} );

Copy link
Member Author

@gziolo gziolo Mar 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@aduth I want to refactor all connect occurrences in the edit-post module next week to see if it has any impact on the module’s bundle size. I’ll follow your advice despite the fact that I personally like this short notation 😃

isPublishSidebarOpened: select( 'core/edit-post' ).isPublishSidebarOpened(),
hasActiveMetaboxes: select( 'core/edit-post' ).hasMetaBoxes(),
isSaving: select( 'core/edit-post' ).isSavingMetaBoxes(),
} ) ),
withDispatch( ( dispatch ) => ( {
openGeneralSidebar: () => dispatch( 'core/edit-post' ).openGeneralSidebar( 'edit-post/document' ),
closeGeneralSidebar: dispatch( 'core/edit-post' ).closeGeneralSidebar,
togglePublishSidebar: dispatch( 'core/edit-post' ).togglePublishSidebar,
} ) ),
)( Header );
84 changes: 27 additions & 57 deletions edit-post/components/layout/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/**
* External dependencies
*/
import { connect } from 'react-redux';
import classnames from 'classnames';
import { some } from 'lodash';

Expand All @@ -17,6 +16,8 @@ import {
PostPublishPanel,
DocumentTitle,
} from '@wordpress/editor';
import { withDispatch, withSelect } from '@wordpress/data';
import { compose } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -29,48 +30,21 @@ import VisualEditor from '../visual-editor';
import EditorModeKeyboardShortcuts from '../keyboard-shortcuts';
import MetaBoxes from '../meta-boxes';
import { getMetaBoxContainer } from '../../utils/meta-boxes';
import {
getEditorMode,
hasOpenSidebar,
isFeatureActive,
getOpenedGeneralSidebar,
isPublishSidebarOpened,
getActivePlugin,
getMetaBoxes,
hasMetaBoxes,
isSavingMetaBoxes,
} from '../../store/selectors';
import { closePublishSidebar } from '../../store/actions';
import PluginsPanel from '../../components/plugins-panel/index.js';
import { getSidebarSettings } from '../../api/sidebar';

function GeneralSidebar( { openedGeneralSidebar } ) {
switch ( openedGeneralSidebar ) {
case 'editor':
return <Sidebar />;
case 'plugin':
return <PluginsPanel />;
default:
}
return null;
}

function Layout( {
mode,
layoutHasOpenSidebar,
publishSidebarOpen,
openedGeneralSidebar,
editorSidebarOpened,
pluginSidebarOpened,
publishSidebarOpened,
hasFixedToolbar,
onClosePublishSidebar,
plugin,
closePublishSidebar,
metaBoxes,
hasActiveMetaboxes,
isSaving,
} ) {
const isSidebarOpened = layoutHasOpenSidebar &&
( openedGeneralSidebar !== 'plugin' || getSidebarSettings( plugin ) );
const className = classnames( 'edit-post-layout', {
'is-sidebar-opened': isSidebarOpened,
'is-sidebar-opened': editorSidebarOpened || pluginSidebarOpened || publishSidebarOpened,
'has-fixed-toolbar': hasFixedToolbar,
} );

Expand Down Expand Up @@ -99,37 +73,33 @@ function Layout( {
<MetaBoxes location="advanced" />
</div>
</div>
{ publishSidebarOpen && (
{ publishSidebarOpened && (
<PostPublishPanel
onClose={ onClosePublishSidebar }
onClose={ closePublishSidebar }
forceIsDirty={ hasActiveMetaboxes }
forceIsSaving={ isSaving }
/>
) }
{
openedGeneralSidebar !== null && <GeneralSidebar
openedGeneralSidebar={ openedGeneralSidebar } />
}
{ editorSidebarOpened && <Sidebar /> }
{ pluginSidebarOpened && <PluginsPanel /> }
<Popover.Slot />
</div>
);
}

export default connect(
( state ) => ( {
mode: getEditorMode( state ),
layoutHasOpenSidebar: hasOpenSidebar( state ),
openedGeneralSidebar: getOpenedGeneralSidebar( state ),
publishSidebarOpen: isPublishSidebarOpened( state ),
hasFixedToolbar: isFeatureActive( state, 'fixedToolbar' ),
plugin: getActivePlugin( state ),
metaBoxes: getMetaBoxes( state ),
hasActiveMetaboxes: hasMetaBoxes( state ),
isSaving: isSavingMetaBoxes( state ),
} ),
{
onClosePublishSidebar: closePublishSidebar,
},
undefined,
{ storeKey: 'edit-post' }
)( navigateRegions( Layout ) );
export default compose(
withSelect( ( select ) => ( {
mode: select( 'core/edit-post' ).getEditorMode(),
editorSidebarOpened: select( 'core/edit-post' ).isEditorSidebarOpened(),
pluginSidebarOpened: select( 'core/edit-post' ).isPluginSidebarOpened(),
publishSidebarOpened: select( 'core/edit-post' ).isPublishSidebarOpened(),
hasFixedToolbar: select( 'core/edit-post' ).isFeatureActive( 'fixedToolbar' ),
metaBoxes: select( 'core/edit-post' ).getMetaBoxes(),
hasActiveMetaboxes: select( 'core/edit-post' ).hasMetaBoxes(),
isSaving: select( 'core/edit-post' ).isSavingMetaBoxes(),
} ) ),
withDispatch( ( dispatch ) => ( {
closePublishSidebar: dispatch( 'core/edit-post' ).closePublishSidebar,
} ) ),
navigateRegions
)( Layout );
33 changes: 12 additions & 21 deletions edit-post/components/plugins-panel/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,18 @@
/**
* External dependencies
*/
import { connect } from 'react-redux';

/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { IconButton, withFocusReturn } from '@wordpress/components';
import { withDispatch, withSelect } from '@wordpress/data';
import { compose } from '@wordpress/element';

/**
* Internal Dependencies
*/
import './style.scss';
import { getSidebarSettings } from '../../api/sidebar';
import { getActivePlugin } from '../../store/selectors';
import { closeGeneralSidebar } from '../../store/actions';

function PluginsPanel( { onClose, plugin } ) {
const pluginSidebar = getSidebarSettings( plugin );

function PluginsPanel( { onClose, pluginSidebar } ) {
if ( ! pluginSidebar ) {
return null;
}
Expand Down Expand Up @@ -50,14 +43,12 @@ function PluginsPanel( { onClose, plugin } ) {
);
}

export default connect(
( state ) => {
return {
plugin: getActivePlugin( state ),
};
}, {
onClose: closeGeneralSidebar,
},
undefined,
{ storeKey: 'edit-post' }
)( withFocusReturn( PluginsPanel ) );
export default compose(
withSelect( ( select ) => ( {
pluginSidebar: getSidebarSettings( select( 'core/edit-post' ).getActiveGeneralSidebarName() ),
} ) ),
withDispatch( ( dispatch ) => ( {
onClose: dispatch( 'core/edit-post' ).closeGeneralSidebar,
} ) ),
withFocusReturn,
)( PluginsPanel );
41 changes: 12 additions & 29 deletions edit-post/components/sidebar/header.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,44 +1,33 @@
/**
* External dependencies
*/
import { connect } from 'react-redux';

/**
* WordPress dependencies
*/
import { compose } from '@wordpress/element';
import { __, _n, sprintf } from '@wordpress/i18n';
import { IconButton } from '@wordpress/components';
import { withSelect } from '@wordpress/data';
import { withDispatch, withSelect } from '@wordpress/data';

/**
* Internal Dependencies
*/
import { getActiveEditorPanel } from '../../store/selectors';
import { closeGeneralSidebar, setGeneralSidebarActivePanel } from '../../store/actions';

const SidebarHeader = ( { panel, onSetPanel, onCloseSidebar, count } ) => {
const SidebarHeader = ( { activeSidebarName, openSidebar, closeSidebar, count } ) => {
// Do not display "0 Blocks".
count = count === 0 ? 1 : count;

return (
<div className="components-panel__header edit-post-sidebar__panel-tabs">
<button
onClick={ () => onSetPanel( 'document' ) }
className={ `edit-post-sidebar__panel-tab ${ panel === 'document' ? 'is-active' : '' }` }
onClick={ () => openSidebar( 'edit-post/document' ) }
className={ `edit-post-sidebar__panel-tab ${ activeSidebarName === 'edit-post/document' ? 'is-active' : '' }` }
aria-label={ __( 'Document settings' ) }
>
{ __( 'Document' ) }
</button>
<button
onClick={ () => onSetPanel( 'block' ) }
className={ `edit-post-sidebar__panel-tab ${ panel === 'block' ? 'is-active' : '' }` }
onClick={ () => openSidebar( 'edit-post/block' ) }
className={ `edit-post-sidebar__panel-tab ${ activeSidebarName === 'edit-post/block' ? 'is-active' : '' }` }
aria-label={ __( 'Block settings' ) }
>
{ sprintf( _n( 'Block', '%d Blocks', count ), count ) }
</button>
<IconButton
onClick={ onCloseSidebar }
onClick={ closeSidebar }
icon="no-alt"
label={ __( 'Close settings' ) }
/>
Expand All @@ -49,16 +38,10 @@ const SidebarHeader = ( { panel, onSetPanel, onCloseSidebar, count } ) => {
export default compose(
withSelect( ( select ) => ( {
count: select( 'core/editor' ).getSelectedBlockCount(),
activeSidebarName: select( 'core/edit-post' ).getActiveGeneralSidebarName(),
} ) ),
withDispatch( ( dispatch ) => ( {
openSidebar: dispatch( 'core/edit-post' ).openGeneralSidebar,
closeSidebar: dispatch( 'core/edit-post' ).closeGeneralSidebar,
} ) ),
connect(
( state ) => ( {
panel: getActiveEditorPanel( state ),
} ),
{
onSetPanel: setGeneralSidebarActivePanel.bind( null, 'editor' ),
onCloseSidebar: closeGeneralSidebar,
},
undefined,
{ storeKey: 'edit-post' }
)
)( SidebarHeader );
Loading