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

Editor: Persist the selected editor mode across reloads #2568

Merged
merged 6 commits into from
Aug 29, 2017

Conversation

youknowriad
Copy link
Contributor

closes #450

Testing instructions

  • Switch to the text mode
  • Refresh the page
  • You should stay in text mode

@youknowriad youknowriad self-assigned this Aug 28, 2017
@codecov
Copy link

codecov bot commented Aug 28, 2017

Codecov Report

Merging #2568 into master will increase coverage by 1%.
The diff coverage is 40%.

Impacted file tree graph

@@           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
Impacted Files Coverage Δ
editor/sidebar/post-excerpt/index.js 0% <0%> (ø) ⬆️
editor/sidebar/table-of-contents/index.js 0% <0%> (ø) ⬆️
editor/sidebar/post-status/index.js 0% <0%> (ø) ⬆️
editor/sidebar/post-taxonomies/index.js 0% <0%> (ø) ⬆️
editor/sidebar/discussion-panel/index.js 0% <0%> (ø) ⬆️
editor/sidebar/featured-image/index.js 0% <0%> (ø) ⬆️
editor/reducer.js 88.65% <100%> (-0.16%) ⬇️
editor/selectors.js 96.4% <100%> (+0.02%) ⬆️
editor/sidebar/page-attributes/index.js 75% <50%> (-1.2%) ⬇️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6143210...80c3ffe. Read the comment docs.

@youknowriad youknowriad changed the title Editor: Persist the selected editor mode accross reloads Editor: Persist the selected editor mode across reloads Aug 28, 2017
@@ -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".
Copy link
Member

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:

http://usejsdoc.org/tags-type.html#jsdoc-types

@@ -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';
Copy link
Member

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)?

@@ -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 }, {
Copy link
Member

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 }>
Copy link
Member

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 }
	>

@youknowriad youknowriad merged commit 09380a4 into master Aug 29, 2017
@youknowriad youknowriad deleted the update/persist-editor-mode branch August 29, 2017 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Save editor UI configuration to be persistent across reloads
2 participants