Skip to content

Commit

Permalink
Components: Preserve fill ordering through unmounts
Browse files Browse the repository at this point in the history
  • Loading branch information
aduth committed Nov 16, 2017
1 parent 3d54d33 commit 73c3e30
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 30 deletions.
6 changes: 4 additions & 2 deletions blocks/block-edit/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ function BlockEdit( props ) {
// `edit` and `save` are functions or components describing the markup
// with which a block is displayed. If `blockType` is valid, assign
// them preferencially as the render value for the block.
const Edit = blockType.edit || blockType.save;
Edit.displayName = 'Edit';
let Edit;
if ( blockType ) {
Edit = blockType.edit || blockType.save;
}

return applyFilters( 'BlockEdit', <Edit key="edit" { ...editProps } />, props );
}
Expand Down
9 changes: 4 additions & 5 deletions blocks/hooks/anchor.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { assign } from 'lodash';
/**
* WordPress dependencies
*/
import { concatChildren } from '@wordpress/element';
import { __ } from '@wordpress/i18n';

/**
Expand Down Expand Up @@ -55,9 +54,9 @@ export function addAttribute( settings ) {
*/
export function addInspectorControl( element, props ) {
if ( hasBlockSupport( props.name, 'anchor' ) && props.focus ) {
element = concatChildren(
element = [
element,
<InspectorControls>
<InspectorControls key="inspector-anchor">
<InspectorControls.TextControl
label={ __( 'HTML Anchor' ) }
help={ __( 'Anchors lets you link directly to a section on a page.' ) }
Expand All @@ -69,8 +68,8 @@ export function addInspectorControl( element, props ) {
anchor: nextValue,
} );
} } />
</InspectorControls>
);
</InspectorControls>,
];
}

return element;
Expand Down
9 changes: 4 additions & 5 deletions blocks/hooks/custom-class-name.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import classnames from 'classnames';
/**
* WordPress dependencies
*/
import { concatChildren } from '@wordpress/element';
import { __ } from '@wordpress/i18n';

/**
Expand Down Expand Up @@ -46,9 +45,9 @@ export function addAttribute( settings ) {
*/
export function addInspectorControl( element, props ) {
if ( hasBlockSupport( props.name, 'customClassName', true ) && props.focus ) {
element = concatChildren(
element = [
element,
<InspectorControls force>
<InspectorControls key="inspector-custom-classname">
<InspectorControls.TextControl
label={ __( 'Additional CSS Class' ) }
value={ props.attributes.className || '' }
Expand All @@ -58,8 +57,8 @@ export function addInspectorControl( element, props ) {
} );
} }
/>
</InspectorControls>
);
</InspectorControls>,
];
}

return element;
Expand Down
4 changes: 2 additions & 2 deletions blocks/inspector-controls/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import TextControl from './text-control';
import TextareaControl from './textarea-control';
import ToggleControl from './toggle-control';

export default function InspectorControls( { force = false, children } ) {
export default function InspectorControls( { children } ) {
return (
<Fill name="Inspector.Controls" force={ force }>
<Fill name="Inspector.Controls">
{ children }
</Fill>
);
Expand Down
25 changes: 18 additions & 7 deletions components/slot-fill/fill.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,32 +8,39 @@ import { noop } from 'lodash';
*/
import { Component, createPortal } from '@wordpress/element';

/**
* Internal dependencies
*/
import withInstanceId from '../higher-order/with-instance-id';
let occurrences = 0;

class Fill extends Component {
componentWillMount() {
this.occurrence = ++occurrences;
}

componentDidMount() {
const { registerFill = noop } = this.context;

registerFill( this.props.name, this );
}

componentWillUpdate() {
if ( ! this.occurrence ) {
this.occurrence = ++occurrences;
}
}

componentWillUnmount() {
const { unregisterFill = noop } = this.context;

unregisterFill( this.props.name, this );
}

componentWillReceiveProps( nextProps ) {
const { name, force } = nextProps;
const { name } = nextProps;
const {
unregisterFill = noop,
registerFill = noop,
} = this.context;

if ( this.props.name !== name || force ) {
if ( this.props.name !== name ) {
unregisterFill( this.props.name, this );
registerFill( name, this );
}
Expand All @@ -47,6 +54,10 @@ class Fill extends Component {
}
}

resetOccurrence() {
this.occurrence = null;
}

render() {
const { getSlot = noop } = this.context;
const { name, children } = this.props;
Expand All @@ -65,4 +76,4 @@ Fill.contextTypes = {
unregisterFill: noop,
};

export default withInstanceId( Fill );
export default Fill;
19 changes: 12 additions & 7 deletions components/slot-fill/provider.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { pick, without, noop } from 'lodash';
import { pick, sortBy, forEach, without, noop } from 'lodash';

/**
* WordPress dependencies
Expand Down Expand Up @@ -61,6 +61,7 @@ class SlotFillProvider extends Component {
this.fills[ name ],
instance
);
this.resetFillOccurrence( name );
this.forceUpdateSlot( name );
}

Expand All @@ -69,15 +70,19 @@ class SlotFillProvider extends Component {
}

getFills( name ) {
return this.fills[ name ];
return sortBy( this.fills[ name ], 'occurrence' );
}

resetFillOccurrence( name ) {
forEach( this.fills[ name ], ( instance ) => {
instance.resetOccurrence();
} );
}

forceUpdateFills( name ) {
if ( this.fills.hasOwnProperty( name ) ) {
this.fills[ name ].forEach( ( instance ) => {
instance.forceUpdate();
} );
}
forEach( this.fills[ name ], ( instance ) => {
instance.forceUpdate();
} );
}

forceUpdateSlot( name ) {
Expand Down
2 changes: 1 addition & 1 deletion components/slot-fill/slot.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class Slot extends Component {
return (
<div ref={ this.bindNode }>
{ map( getFills( name ), ( fill ) => {
const fillKey = fill.props.instanceId;
const fillKey = fill.occurrence;
return Children.map( fill.props.children, ( child, childIndex ) => {
if ( ! child || isString( child ) ) {
return child;
Expand Down
35 changes: 34 additions & 1 deletion components/slot-fill/test/slot.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class Filler extends Component {
render() {
return [
<button key="1" type="button" onClick={ () => this.setState( { num: this.state.num + 1 } ) } />,
<Fill name={ this.props.name } key="2">{ this.state.num.toString() }</Fill>,
<Fill name={ this.props.name } key="2">{ this.props.text || this.state.num.toString() }</Fill>,
];
}
}
Expand Down Expand Up @@ -96,4 +96,37 @@ describe( 'Slot', () => {

expect( element.find( 'Slot > div' ).html() ).toBe( '<div>2</div>' );
} );

it( 'should render in expected order', () => {
const element = mount(
<Provider>
<Slot name="egg" key="slot" />
</Provider>
);

element.setProps( {
children: [
<Slot name="egg" key="slot" />,
<Filler name="egg" key="first" text="first" />,
<Filler name="egg" key="second" text="second" />,
],
} );

element.setProps( {
children: [
<Slot name="egg" key="slot" />,
<Filler name="egg" key="second" text="second" />,
],
} );

element.setProps( {
children: [
<Slot name="egg" key="slot" />,
<Filler name="egg" key="first" text="first" />,
<Filler name="egg" key="second" text="second" />,
],
} );

expect( element.find( 'Slot > div' ).html() ).toBe( '<div>firstsecond</div>' );
} );
} );

0 comments on commit 73c3e30

Please sign in to comment.