-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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 ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any need for There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly, not sure we need to test presence of |
||
} | ||
|
||
/** | ||
* Returns true if any past editor history snapshots exist, or false otherwise. | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -67,17 +74,17 @@ const applyConnect = connect( | |
( state ) => { | ||
return { | ||
featuredImageId: getEditedPostAttribute( state, 'featured_media' ), | ||
isOpened: isEditorSidebarPanelOpened( state, PANEL_NAME ), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wondering if we should bind props using |
||
}; | ||
}, | ||
( 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, | ||
} | ||
); | ||
|
||
|
There was a problem hiding this comment.
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
.There was a problem hiding this comment.
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.