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

Chrome: Persist the "opened/closed" state of the sidebar panels in localStorage #2533

Merged
merged 1 commit into from
Aug 28, 2017
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
24 changes: 15 additions & 9 deletions components/panel/body.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,22 @@ class PanelBody extends Component {

toggle( event ) {
event.preventDefault();
this.setState( ( state ) => ( {
opened: ! state.opened,
} ) );
if ( this.props.opened === undefined ) {
this.setState( ( state ) => ( {
opened: ! state.opened,
} ) );
}

if ( this.props.onToggle ) {
this.props.onToggle();
}
}

render() {
const { title, children } = this.props;
const { opened } = this.state;
const icon = `arrow-${ opened ? 'down' : 'right' }`;
const className = classnames( 'components-panel__body', { 'is-opened': opened } );
const { title, children, opened } = this.props;
const isOpened = opened === undefined ? this.state.opened : opened;
Copy link
Member

Choose a reason for hiding this comment

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

Are there any places this component is used with the state variant now? Wonder if we should just treat it as a controlled component, rather than try to juggle both state and prop values of opened.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not right now, no strong preference.

const icon = `arrow-${ isOpened ? 'down' : 'right' }`;
const className = classnames( 'components-panel__body', { 'is-opened': isOpened } );

return (
<div className={ className }>
Expand All @@ -44,15 +50,15 @@ class PanelBody extends Component {
<Button
className="components-panel__body-toggle"
onClick={ this.toggle }
aria-expanded={ opened }
aria-expanded={ isOpened }
label={ sprintf( __( 'Open section: %s' ), title ) }
>
<Dashicon icon={ icon } />
{ title }
</Button>
</h3>
) }
{ this.state.opened && children }
{ isOpened && children }
</div>
);
}
Expand Down
7 changes: 7 additions & 0 deletions components/panel/test/body.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ describe( 'PanelBody', () => {
expect( icon.prop( 'icon' ) ).toBe( 'arrow-right' );
} );

it( 'should use the "opened" prop instead of state if provided', () => {
const panelBody = shallow( <PanelBody title="Some Text" opened={ true } initialOpen={ false } /> );
expect( panelBody.state( 'opened' ) ).toBe( false );
const icon = panelBody.find( 'Dashicon' );
expect( icon.prop( 'icon' ) ).toBe( 'arrow-down' );
} );

it( 'should render child elements within PanelBody element', () => {
const panelBody = shallow( <PanelBody children="Some Text" /> );
expect( panelBody.instance().props.children ).toBe( 'Some Text' );
Expand Down
13 changes: 13 additions & 0 deletions editor/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,19 @@ export function stopTyping() {
};
}

/**
* Returns an action object used in signalling that the user toggled a sidebar panel
*
* @param {String} panel The panel name
* @return {Object} Action object
*/
export function toggleSidebarPanel( panel ) {
return {
type: 'TOGGLE_SIDEBAR_PANEL',
panel,
};
}

/**
* Returns an action object used to create a notice
*
Expand Down
14 changes: 11 additions & 3 deletions editor/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
import optimist from 'redux-optimist';
import { combineReducers } from 'redux';
import { reduce, keyBy, first, last, omit, without, mapValues } from 'lodash';
import { get, reduce, keyBy, first, last, omit, without, mapValues } from 'lodash';

/**
* WordPress dependencies
Expand Down Expand Up @@ -441,13 +441,21 @@ export function mode( state = 'visual', action ) {
return state;
}

export function preferences( state = { isSidebarOpened: ! isMobile }, action ) {
export function preferences( state = { isSidebarOpened: ! isMobile, panels: { 'post-status': true } }, action ) {
Copy link
Member

Choose a reason for hiding this comment

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

This line is pretty long and we might consider breaking out the default to its own constant variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will fix the notes in #2568

switch ( action.type ) {
case 'TOGGLE_SIDEBAR':
return {
... state,
...state,
isSidebarOpened: ! state.isSidebarOpened,
};
case 'TOGGLE_SIDEBAR_PANEL':
return {
...state,
panels: {
...state.panels,
[ action.panel ]: ! get( state, [ 'panels', action.panel ], false ),
Copy link
Member

Choose a reason for hiding this comment

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

Is there any need for get here? state.panels seems it should always be an object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's needed for users who already have the preferences persisted.

},
};
}

return state;
Expand Down
14 changes: 13 additions & 1 deletion editor/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export function getPreference( state, preferenceKey ) {
}

/**
* Returns true if the editor sidebar panel is open, or false otherwise.
* Returns true if the editor sidebar is open, or false otherwise.
*
* @param {Object} state Global application state
* @return {Boolean} Whether sidebar is open
Expand All @@ -63,6 +63,18 @@ export function isEditorSidebarOpened( state ) {
return getPreference( state, 'isSidebarOpened' );
}

/**
* Returns true if the editor sidebar panel is open, or false otherwise.
*
* @param {Object} state Global application state
* @param {STring} panel Sidebar panel name
* @return {Boolean} Whether sidebar is open
*/
export function isEditorSidebarPanelOpened( state, panel ) {
const panels = getPreference( state, 'panels' );
return panels ? !! panels[ panel ] : false;
Copy link
Member

Choose a reason for hiding this comment

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

Similarly, not sure we need to test presence of panels.

}

/**
* Returns true if any past editor history snapshots exist, or false otherwise.
*
Expand Down
17 changes: 12 additions & 5 deletions editor/sidebar/discussion-panel/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,24 @@ import { PanelBody, PanelRow, FormToggle, withInstanceId } from '@wordpress/comp
/**
* Internal Dependencies
*/
import { getEditedPostAttribute } from '../../selectors';
import { editPost } from '../../actions';
import { getEditedPostAttribute, isEditorSidebarPanelOpened } from '../../selectors';
import { editPost, toggleSidebarPanel } from '../../actions';

function DiscussionPanel( { pingStatus = 'open', commentStatus = 'open', instanceId, ...props } ) {
/**
* Module Constants
*/
const PANEL_NAME = 'discussion-panel';

function DiscussionPanel( { pingStatus = 'open', commentStatus = 'open', instanceId, isOpened, ...props } ) {
const onTogglePingback = () => props.editPost( { ping_status: pingStatus === 'open' ? 'closed' : 'open' } );
const onToggleComments = () => props.editPost( { comment_status: commentStatus === 'open' ? 'closed' : 'open' } );
const onTogglePanel = () => props.toggleSidebarPanel( PANEL_NAME );

const commentsToggleId = 'allow-comments-toggle-' + instanceId;
const pingbacksToggleId = 'allow-pingbacks-toggle-' + instanceId;

return (
<PanelBody title={ __( 'Discussion' ) } initialOpen={ false }>
<PanelBody title={ __( 'Discussion' ) } opened={ isOpened } onToggle={ onTogglePanel }>
<PanelRow>
<label htmlFor={ commentsToggleId }>{ __( 'Allow Comments' ) }</label>
<FormToggle
Expand Down Expand Up @@ -51,8 +57,9 @@ export default connect(
return {
pingStatus: getEditedPostAttribute( state, 'ping_status' ),
commentStatus: getEditedPostAttribute( state, 'comment_status' ),
isOpened: isEditorSidebarPanelOpened( state, PANEL_NAME ),
};
},
{ editPost }
{ editPost, toggleSidebarPanel }
)( withInstanceId( DiscussionPanel ) );

33 changes: 20 additions & 13 deletions editor/sidebar/featured-image/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,19 @@ import { MediaUploadButton } from '@wordpress/blocks';
* Internal dependencies
*/
import './style.scss';
import { getEditedPostAttribute } from '../../selectors';
import { editPost } from '../../actions';
import { getEditedPostAttribute, isEditorSidebarPanelOpened } from '../../selectors';
import { editPost, toggleSidebarPanel } from '../../actions';

/**
* Module Constants
*/
const PANEL_NAME = 'featured-image';

function FeaturedImage( { featuredImageId, onUpdateImage, onRemoveImage, media, isOpened, ...props } ) {
const onToggle = () => props.toggleSidebarPanel( PANEL_NAME );

function FeaturedImage( { featuredImageId, onUpdateImage, onRemoveImage, media } ) {
return (
<PanelBody title={ __( 'Featured image' ) } initialOpen={ false }>
<PanelBody title={ __( 'Featured image' ) } opened={ isOpened } onToggle={ onToggle }>
<div className="editor-featured-image__content">
{ !! featuredImageId &&
<MediaUploadButton
Expand Down Expand Up @@ -67,17 +74,17 @@ const applyConnect = connect(
( state ) => {
return {
featuredImageId: getEditedPostAttribute( state, 'featured_media' ),
isOpened: isEditorSidebarPanelOpened( state, PANEL_NAME ),
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if we should bind props using PANEL_NAME not only in mapStateToProps, but also in mapDispatchToProps, so toggle callback in the visual component doesn't need to know/pass the value.

};
},
( dispatch ) => {
return {
onUpdateImage( image ) {
dispatch( editPost( { featured_media: image.id } ) );
},
onRemoveImage() {
dispatch( editPost( { featured_media: null } ) );
},
};
{
onUpdateImage( image ) {
return editPost( { featured_media: image.id } );
},
onRemoveImage() {
return editPost( { featured_media: null } );
},
toggleSidebarPanel,
}
);

Expand Down
35 changes: 23 additions & 12 deletions editor/sidebar/page-attributes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,20 @@ import { PanelBody, PanelRow, withAPIData, withInstanceId } from '@wordpress/com
/**
* Internal dependencies
*/
import { editPost } from '../../actions';
import { getCurrentPostType, getEditedPostAttribute } from '../../selectors';
import { editPost, toggleSidebarPanel } from '../../actions';
import { getCurrentPostType, getEditedPostAttribute, isEditorSidebarPanelOpened } from '../../selectors';

/**
* Module Constants
*/
const PANEL_NAME = 'page-attributes';

export class PageAttributes extends Component {
constructor() {
super( ...arguments );

this.setUpdatedOrder = this.setUpdatedOrder.bind( this );
this.onToggle = this.onToggle.bind( this );

this.state = {
supportsPageAttributes: false,
Expand All @@ -35,8 +41,12 @@ export class PageAttributes extends Component {
}
}

onToggle() {
this.props.toggleSidebarPanel( PANEL_NAME );
}

render() {
const { instanceId, order, postType } = this.props;
const { instanceId, order, postType, isOpened } = this.props;
const supportsPageAttributes = get( postType.data, [
'supports',
'page-attributes',
Expand All @@ -53,7 +63,8 @@ export class PageAttributes extends Component {
return (
<PanelBody
title={ __( 'Page Attributes' ) }
initialOpen={ false }
opened={ isOpened }
onToggle={ this.onToggle }
>
<PanelRow>
<label htmlFor={ inputId }>
Expand All @@ -76,16 +87,16 @@ const applyConnect = connect(
return {
postTypeSlug: getCurrentPostType( state ),
order: getEditedPostAttribute( state, 'menu_order' ),
isOpened: isEditorSidebarPanelOpened( state, PANEL_NAME ),
};
},
( dispatch ) => {
return {
onUpdateOrder( order ) {
dispatch( editPost( {
menu_order: order,
} ) );
},
};
{
onUpdateOrder( order ) {
return editPost( {
menu_order: order,
} );
},
toggleSidebarPanel,
}
);

Expand Down
26 changes: 16 additions & 10 deletions editor/sidebar/post-excerpt/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,20 @@ import { ExternalLink, PanelBody } from '@wordpress/components';
* Internal Dependencies
*/
import './style.scss';
import { getEditedPostExcerpt } from '../../selectors';
import { editPost } from '../../actions';
import { getEditedPostExcerpt, isEditorSidebarPanelOpened } from '../../selectors';
import { editPost, toggleSidebarPanel } from '../../actions';

function PostExcerpt( { excerpt, onUpdateExcerpt } ) {
/**
* Module Constants
*/
const PANEL_NAME = 'post-excerpt';

function PostExcerpt( { excerpt, onUpdateExcerpt, isOpened, ...props } ) {
const onChange = ( event ) => onUpdateExcerpt( event.target.value );
const onTogglePanel = () => props.toggleSidebarPanel( PANEL_NAME );

return (
<PanelBody title={ __( 'Excerpt' ) } initialOpen={ false }>
<PanelBody title={ __( 'Excerpt' ) } opened={ isOpened } onToggle={ onTogglePanel }>
<textarea
className="editor-post-excerpt__textarea"
onChange={ onChange }
Expand All @@ -39,14 +45,14 @@ export default connect(
( state ) => {
return {
excerpt: getEditedPostExcerpt( state ),
isOpened: isEditorSidebarPanelOpened( state, PANEL_NAME ),
};
},
( dispatch ) => {
return {
onUpdateExcerpt( excerpt ) {
dispatch( editPost( { excerpt } ) );
},
};
{
onUpdateExcerpt( excerpt ) {
return editPost( { excerpt } );
},
toggleSidebarPanel,
}
)( PostExcerpt );

Loading