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

Framework: Hide The Editor Settings from the block author #2119

Merged
merged 2 commits into from
Aug 3, 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
13 changes: 12 additions & 1 deletion blocks/block-alignment-toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
import { __ } from 'i18n';
import { Toolbar } from 'components';

/**
* Internal dependencies
*/
import withEditorSettings from '../with-editor-settings';

const BLOCK_ALIGNMENTS_CONTROLS = {
left: {
icon: 'align-left',
Expand All @@ -30,7 +35,7 @@ const BLOCK_ALIGNMENTS_CONTROLS = {
const DEFAULT_CONTROLS = [ 'left', 'center', 'right', 'wide', 'full' ];
const WIDE_CONTROLS = [ 'wide', 'full' ];

export default function BlockAlignmentToolbar( { value, onChange, controls = DEFAULT_CONTROLS, wideControlsEnabled = false } ) {
function BlockAlignmentToolbar( { value, onChange, controls = DEFAULT_CONTROLS, wideControlsEnabled = false } ) {
function applyOrUnset( align ) {
return () => onChange( value === align ? undefined : align );
}
Expand All @@ -53,3 +58,9 @@ export default function BlockAlignmentToolbar( { value, onChange, controls = DEF
/>
);
}

export default withEditorSettings(
( settings ) => ( {
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need a mapping? Should we just pass all settings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm planning to introduce the blockTypes, fallbackBlockName.... to these settings and I expect several components to extract a single or multiple blockTypes from the settings. Mapping helps use factorize this using functions similar to selectors to avoid duplication.

Also, It's nice that the component prop is wideAlignmentsEnabled instead of leaving the responsibility of extracting the prop to the component itself.

wideControlsEnabled: settings.wideImages,
} )
)( BlockAlignmentToolbar );
3 changes: 1 addition & 2 deletions blocks/library/cover-image/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ registerBlockType( 'core/cover-image', {
}
},

edit( { attributes, setAttributes, focus, setFocus, className, settings } ) {
edit( { attributes, setAttributes, focus, setFocus, className } ) {
const { url, title, align, id, hasParallax, hasBackgroundDim } = attributes;
const updateAlignment = ( nextAlign ) => setAttributes( { align: nextAlign } );
const onSelectImage = ( media ) => setAttributes( { url: media.url, id: media.id } );
Expand All @@ -56,7 +56,6 @@ registerBlockType( 'core/cover-image', {
<BlockAlignmentToolbar
value={ align }
onChange={ updateAlignment }
wideControlsEnabled={ settings.wideImages }
/>

<Toolbar>
Expand Down
3 changes: 1 addition & 2 deletions blocks/library/cover-text/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ registerBlockType( 'core/cover-text', {
};
},

edit( { attributes, setAttributes, className, focus, setFocus, mergeBlocks, settings } ) {
edit( { attributes, setAttributes, className, focus, setFocus, mergeBlocks } ) {
const { align, width, content, dropCap, placeholder, textColor, backgroundColor } = attributes;
const toggleDropCap = () => setAttributes( { dropCap: ! dropCap } );
return [
Expand All @@ -75,7 +75,6 @@ registerBlockType( 'core/cover-text', {
value={ width }
onChange={ ( nextWidth ) => setAttributes( { width: nextWidth } ) }
controls={ [ 'center', 'wide', 'full' ] }
wideControlsEnabled={ settings.wideImages }
/>
<AlignmentToolbar
value={ align }
Expand Down
3 changes: 1 addition & 2 deletions blocks/library/gallery/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ registerBlockType( 'core/gallery', {
}
},

edit( { attributes, setAttributes, focus, className, settings } ) {
edit( { attributes, setAttributes, focus, className } ) {
const { images = [], columns = defaultColumnsNumber( attributes ), align = 'none' } = attributes;
const setColumnsNumber = ( event ) => setAttributes( { columns: event.target.value } );
const updateAlignment = ( nextAlign ) => setAttributes( { align: nextAlign } );
Expand All @@ -90,7 +90,6 @@ registerBlockType( 'core/gallery', {
<BlockAlignmentToolbar
value={ align }
onChange={ updateAlignment }
wideControlsEnabled={ settings.wideImages }
/>
{ !! images.length && (
<Toolbar controls={ [ {
Expand Down
3 changes: 1 addition & 2 deletions blocks/library/image/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ registerBlockType( 'core/image', {
}
},

edit( { attributes, setAttributes, focus, setFocus, className, settings } ) {
edit( { attributes, setAttributes, focus, setFocus, className } ) {
const { url, alt, caption, align, id, href } = attributes;
const updateAlt = ( newAlt ) => setAttributes( { alt: newAlt } );
const updateAlignment = ( nextAlign ) => setAttributes( { align: nextAlign } );
Expand All @@ -79,7 +79,6 @@ registerBlockType( 'core/image', {
<BlockAlignmentToolbar
value={ align }
onChange={ updateAlignment }
wideControlsEnabled={ settings.wideImages }
/>

<Toolbar>
Expand Down
3 changes: 1 addition & 2 deletions blocks/library/latest-posts/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ registerBlockType( 'core/latest-posts', {
latestPosts.splice( this.props.attributes.postsToShow, postsDifference );
}

const { focus, settings } = this.props;
const { focus } = this.props;
const { displayPostDate, align, layout, columns } = this.props.attributes;
const layoutControls = [
{
Expand All @@ -144,7 +144,6 @@ registerBlockType( 'core/latest-posts', {
setAttributes( { align: nextAlign } );
} }
controls={ [ 'center', 'wide', 'full' ] }
wideControlsEnabled={ settings.wideImages }
/>
<Toolbar controls={ layoutControls } />
</BlockControls>
Expand Down
3 changes: 1 addition & 2 deletions blocks/library/pullquote/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ registerBlockType( 'core/pullquote', {
}
},

edit( { attributes, setAttributes, focus, setFocus, className, settings } ) {
edit( { attributes, setAttributes, focus, setFocus, className } ) {
const { value, citation, align } = attributes;
const updateAlignment = ( nextAlign ) => setAttributes( { align: nextAlign } );

Expand All @@ -50,7 +50,6 @@ registerBlockType( 'core/pullquote', {
<BlockAlignmentToolbar
value={ align }
onChange={ updateAlignment }
wideControlsEnabled={ settings.wideImages }
/>
</BlockControls>
),
Expand Down
3 changes: 1 addition & 2 deletions blocks/library/table/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ registerBlockType( 'core/table', {
}
},

edit( { attributes, setAttributes, focus, setFocus, className, settings } ) {
edit( { attributes, setAttributes, focus, setFocus, className } ) {
const { content } = attributes;
const updateAlignment = ( nextAlign ) => setAttributes( { align: nextAlign } );
return [
Expand All @@ -43,7 +43,6 @@ registerBlockType( 'core/table', {
<BlockAlignmentToolbar
value={ attributes.align }
onChange={ updateAlignment }
wideControlsEnabled={ settings.wideImages }
/>
</BlockControls>
),
Expand Down
34 changes: 34 additions & 0 deletions blocks/with-editor-settings/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* External dependencies
*/
import { noop } from 'lodash';

/**
* WordPress dependencies
*/
import { Component } from 'element';

const withEditorSettings = ( mapSettingsToProps ) => ( OriginalComponent ) => {
class WrappedComponent extends Component {
render() {
const extraProps = mapSettingsToProps
? mapSettingsToProps( this.context.editor, this.props )
: this.context.editor;

return (
<OriginalComponent
{ ...this.props }
{ ...extraProps }
/>
);
}
}

WrappedComponent.contextTypes = {
editor: noop,
};

return WrappedComponent;
};

export default withEditorSettings;
5 changes: 4 additions & 1 deletion editor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import './assets/stylesheets/main.scss';
import Layout from './layout';
import { createReduxStore } from './state';
import { undo } from './actions';
import EditorSettingsProvider from './settings/provider';

/**
* The default editor settings
Expand Down Expand Up @@ -107,7 +108,9 @@ export function createEditorInstance( id, post, editorSettings = DEFAULT_SETTING
onUndo: undo,
}, store.dispatch ) }
>
<Layout settings={ editorSettings } />
<EditorSettingsProvider settings={ editorSettings }>
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we can start thinking of a nicer way to compose all these providers...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, maybe composeProviders or something :)

Maybe if we extract the editor, we could reduce the number of providers into one (inside the generic editor component) and provide an HoC like withEditorData instead of withEditorSettings to retrieve the editor settings/editor local state from any component. (just an idea we could explore later, not sure if it's a good one yet)

<Layout />
</EditorSettingsProvider>
</EditableProvider>
</SlotFillProvider>
</ReduxProvider>,
Expand Down
5 changes: 1 addition & 4 deletions editor/modes/visual-editor/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import {
isFirstMultiSelectedBlock,
isTyping,
getMultiSelectedBlockUids,
getEditorSettings,
} from '../../selectors';

function FirstChild( { children } ) {
Expand Down Expand Up @@ -305,7 +304,7 @@ class VisualEditorBlock extends Component {
}

render() {
const { block, multiSelectedBlockUids, settings } = this.props;
const { block, multiSelectedBlockUids } = this.props;
const { name: blockName, isValid } = block;
const blockType = getBlockType( blockName );
// translators: %s: Type of block (i.e. Text, Image etc)
Expand Down Expand Up @@ -415,7 +414,6 @@ class VisualEditorBlock extends Component {
setFocus={ partial( onFocus, block.uid ) }
mergeBlocks={ this.mergeBlocks }
className={ className }
settings={ settings }
id={ block.uid }
/>
) }
Expand Down Expand Up @@ -447,7 +445,6 @@ export default connect(
isTyping: isTyping( state ),
order: getBlockIndex( state, ownProps.uid ),
multiSelectedBlockUids: getMultiSelectedBlockUids( state ),
settings: getEditorSettings( state ),
};
},
( dispatch, ownProps ) => ( {
Expand Down
10 changes: 0 additions & 10 deletions editor/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -715,13 +715,3 @@ export function getRecentlyUsedBlocks( state ) {
// resolves the block names in the state to the block type settings
return state.userData.recentlyUsedBlocks.map( blockType => getBlockType( blockType ) );
}

/**
* Get the editor settings.
*
* @param {Object} state Global application state
* @return {Object} The editor settings
*/
export function getEditorSettings( state ) {
return state.settings;
}
30 changes: 30 additions & 0 deletions editor/settings/provider.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* External dependencies
*/
import { noop } from 'lodash';

/**
* WordPress dependencies
*/
import { Component } from 'element';

/**
* The Editor Settings Provider allows any compoent to access the editor settings
*/
class EditorSettingsProvider extends Component {
getChildContext() {
return {
editor: this.props.settings,
};
}

render() {
return this.props.children;
}
}

EditorSettingsProvider.childContextTypes = {
editor: noop,
};

export default EditorSettingsProvider;
10 changes: 0 additions & 10 deletions editor/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -529,15 +529,6 @@ export function notices( state = {}, action ) {
return state;
}

export function settings( state = {}, action ) {
switch ( action.type ) {
case 'SETUP_EDITOR':
return action.settings;
}

return state;
}

/**
* Creates a new instance of a Redux store.
*
Expand All @@ -558,7 +549,6 @@ export function createReduxStore() {
saving,
notices,
userData,
settings,
} ) );

const enhancers = [ applyMiddleware( refx( effects ) ) ];
Expand Down
15 changes: 0 additions & 15 deletions editor/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ import {
didPostSaveRequestFail,
getSuggestedPostFormat,
getNotices,
getEditorSettings,
} from '../selectors';

/**
Expand Down Expand Up @@ -1336,18 +1335,4 @@ describe( 'selectors', () => {
] );
} );
} );

describe( 'getEditorSettings', () => {
it( 'should return the settings object', () => {
const state = {
settings: {
wideImages: true,
},
};

expect( getEditorSettings( state ) ).toEqual( {
wideImages: true,
} );
} );
} );
} );
12 changes: 0 additions & 12 deletions editor/test/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {
showInsertionPoint,
createReduxStore,
userData,
settings,
} from '../state';

describe( 'state', () => {
Expand Down Expand Up @@ -1088,15 +1087,4 @@ describe( 'state', () => {
expect( initial.recentlyUsedBlocks ).toEqual( expect.arrayContaining( [ 'core/test-block', 'core/text' ] ) );
} );
} );

describe( 'settings', () => {
it( 'should populate the editor settings', () => {
const state = settings( {}, {
type: 'SETUP_EDITOR',
settings: { wideImages: true },
} );

expect( state ).toEqual( { wideImages: true } );
} );
} );
} );