-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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: Persist the selected editor mode across reloads #2568
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2568 +/- ##
========================================
+ Coverage 31.09% 32.09% +1%
========================================
Files 174 175 +1
Lines 5278 5381 +103
Branches 905 934 +29
========================================
+ Hits 1641 1727 +86
- Misses 3088 3101 +13
- Partials 549 553 +4
Continue to review full report at Codecov.
|
editor/reducer.js
Outdated
@@ -426,22 +433,16 @@ export function showInsertionPoint( state = false, action ) { | |||
} | |||
|
|||
/** | |||
* Reducer returning current editor mode, either "visual" or "text". | |||
* Reducer returning the user preferences: | |||
* - current editor mode, either "visual" or "text". |
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.
Minor: Should we prefix these list items with the names of the keys they correspond to, e.g.
* - mode: current edidtor mode, either "visual" or "text".
Alternatively, it's also possible to document object shapes with JSDoc:
editor/selectors.js
Outdated
@@ -19,7 +19,7 @@ import { addQueryArgs } from '@wordpress/url'; | |||
* @return {String} Editing mode | |||
*/ | |||
export function getEditorMode( state ) { | |||
return state.mode; | |||
return state.preferences.mode || 'visual'; |
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.
Should we use getPreference( state 'mode' )
here? Is defaulting something which we might consider building into the getPreference
selector (maybe a third argument)?
editor/test/reducer.js
Outdated
@@ -833,6 +815,15 @@ describe( 'state', () => { | |||
|
|||
expect( state ).toEqual( { isSidebarOpened: false, panels: { 'post-taxonomies': false } } ); | |||
} ); | |||
|
|||
it( 'should return switched mode', () => { | |||
const state = preferences( { isSidebarOpened: false }, { |
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.
We ought to deepFreeze
the original state in case of unintended mutations.
@@ -60,7 +54,7 @@ class PostTaxonomies extends Component { | |||
} | |||
|
|||
return ( | |||
<PanelBody title={ __( 'Categories & Tags' ) } opened={ this.props.isOpened } onToggle={ this.onToggle }> | |||
<PanelBody title={ __( 'Categories & Tags' ) } opened={ this.props.isOpened } onToggle={ this.props.onTogglePanel }> |
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.
Minor: Long line could do for either splitting onto multiple lines and/or destructuring props:
const { isOpened, onTogglePanel } = this.props;
return (
<PanelBody
title={ __( 'Categories & Tags' ) }
opened={ isOpened }
onToggle={ onTogglePanel }
>
closes #450
Testing instructions