Skip to content

Commit

Permalink
Components: Handle multiple Slots by same name
Browse files Browse the repository at this point in the history
  • Loading branch information
aduth committed Dec 14, 2018
1 parent a617aee commit 6294dda
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 4 deletions.
26 changes: 24 additions & 2 deletions packages/components/src/slot-fill/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,21 @@ class SlotFillProvider extends Component {
}

registerSlot( name, slot ) {
const previousSlot = this.slots[ name ];
this.slots[ name ] = slot;
this.forceUpdateFills( name );

// Sometimes the fills are registered after the initial render of slot
// But before the registerSlot call, we need to rerender the slot
this.forceUpdateSlot( name );

// If a new instance of a slot is being mounted while another with the
// same name exists, force its update _after_ the new slot has been
// assigned into the instance, such that its own rendering of children
// will be empty (the new Slot will subsume all fills for this name).
if ( previousSlot ) {
previousSlot.forceUpdate();
}
}

registerFill( name, instance ) {
Expand All @@ -57,7 +66,14 @@ class SlotFillProvider extends Component {
this.forceUpdateSlot( name );
}

unregisterSlot( name ) {
unregisterSlot( name, instance ) {
// If a previous instance of a Slot by this name unmounts, do nothing,
// as the slot and its fills should only be removed for the current
// known instance.
if ( this.slots[ name ] !== instance ) {
return;
}

delete this.slots[ name ];
this.forceUpdateFills( name );
}
Expand All @@ -75,7 +91,13 @@ class SlotFillProvider extends Component {
return this.slots[ name ];
}

getFills( name ) {
getFills( name, slotInstance ) {
// Fills should only be returned for the current instance of the slot
// in which they occupy.
if ( this.slots[ name ] !== slotInstance ) {
return [];
}

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

Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/slot-fill/fill.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class FillComponent extends Component {
let { children } = this.props;
const slot = getSlot( name );

if ( ! slot || ! slot.props.bubblesVirtually ) {
if ( ! slot || ! slot.node || ! slot.props.bubblesVirtually ) {
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/slot-fill/slot.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class SlotComponent extends Component {
return <div ref={ this.bindNode } />;
}

const fills = map( getFills( name ), ( fill ) => {
const fills = map( getFills( name, this ), ( fill ) => {
const fillKey = fill.occurrence;
const fillChildren = isFunction( fill.props.children ) ? fill.props.children( fillProps ) : fill.props.children;

Expand Down
54 changes: 54 additions & 0 deletions packages/components/src/slot-fill/test/__snapshots__/slot.js.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,59 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Slot bubblesVirtually false should subsume another slot by the same name 1`] = `
Array [
<div
data-position="first"
/>,
<div
data-position="second"
>
Content
</div>,
]
`;

exports[`Slot bubblesVirtually false should subsume another slot by the same name 2`] = `
Array [
<div
data-position="first"
/>,
<div
data-position="second"
>
Content
</div>,
]
`;

exports[`Slot bubblesVirtually true should subsume another slot by the same name 1`] = `
Array [
<div
data-position="first"
>
<div />
</div>,
<div
data-position="second"
>
<div />
</div>,
]
`;

exports[`Slot bubblesVirtually true should subsume another slot by the same name 2`] = `
Array [
<div
data-position="first"
/>,
<div
data-position="second"
>
<div />
</div>,
]
`;

exports[`Slot should re-render Slot when not bubbling virtually 1`] = `
Array [
<div>
Expand Down
44 changes: 44 additions & 0 deletions packages/components/src/slot-fill/test/slot.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,4 +209,48 @@ describe( 'Slot', () => {

expect( testRenderer.toJSON() ).toMatchSnapshot();
} );

[ false, true ].forEach( ( bubblesVirtually ) => {
describe( 'bubblesVirtually ' + bubblesVirtually, () => {
it( 'should subsume another slot by the same name', () => {
const testRenderer = ReactTestRenderer.create(
<Provider>
<div data-position="first">
<Slot name="egg" bubblesVirtually={ bubblesVirtually } />
</div>
<div data-position="second"></div>
<Fill name="egg">Content</Fill>
</Provider>
);

testRenderer.update(
<Provider>
<div data-position="first">
<Slot name="egg" bubblesVirtually={ bubblesVirtually } />
</div>
<div data-position="second">
<Slot name="egg" bubblesVirtually={ bubblesVirtually } />
</div>
<Fill name="egg">Content</Fill>
</Provider>
);

expect( testRenderer.toJSON() ).toMatchSnapshot();

testRenderer.update(
<Provider>
<div data-position="first"></div>
<div data-position="second">
<Slot name="egg" bubblesVirtually={ bubblesVirtually } />
</div>
<Fill name="egg">Content</Fill>
</Provider>
);

expect( testRenderer.toJSON() ).toMatchSnapshot();

expect( testRenderer.getInstance().slots ).toHaveProperty( 'egg' );
} );
} );
} );
} );

0 comments on commit 6294dda

Please sign in to comment.