Skip to content

Commit

Permalink
Allow EuiContextMenuPanels to update when their children changes (#710)
Browse files Browse the repository at this point in the history 
* Allow EuiContextMenuPanels to update when their children changes

* add PR to changelog

* clarify purpose of shouldComponentUpdate and test code

* comment why expect.assertions call
  • Loading branch information
@chandlerprall
chandlerprall authored May 1, 2018
1 parent becf30a commit d5178c7
Show file tree
Hide file tree
Showing 4 changed files with 226 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
- Added `EuiTabbedContent` ([#737](https://github.com/elastic/eui/pull/737))

**Bug fixes**

- Fixed `EuiTableRowCell` from overwriting its child element's `className` [#709](https://github.com/elastic/eui/pull/709)
- Allow `EuiContextMenuPanel`s to update when their `children` changes ([#710](https://github.com/elastic/eui/pull/710))

## [`0.0.44`](https://github.com/elastic/eui/tree/v0.0.44)

Expand Down
128 changes: 128 additions & 0 deletions src/components/context_menu/__snapshots__/context_menu_panel.test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,131 @@ exports[`EuiContextMenuPanel props transitionDirection previous with transitionT
<div />
</div>
`;

exports[`EuiContextMenuPanel updating items and content updates to items should not re-render if any items's watchedItemProps did not change 1`] = `
"<EuiContextMenuPanel items={{...}} hasFocus={true}>
<div className=\\"euiContextMenuPanel\\" onKeyDown={[Function]} tabIndex=\\"0\\" onAnimationEnd={[Function]}>
<div>
<EuiContextMenuItem data-counter={0} buttonRef={[Function: bound ]}>
<button className=\\"euiContextMenuItem\\" type=\\"button\\" disabled={[undefined]} data-counter={0}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option A
</span>
</span>
</button>
</EuiContextMenuItem>
<EuiContextMenuItem data-counter={1} buttonRef={[Function: bound ]}>
<button className=\\"euiContextMenuItem\\" type=\\"button\\" disabled={[undefined]} data-counter={1}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option B
</span>
</span>
</button>
</EuiContextMenuItem>
</div>
</div>
</EuiContextMenuPanel>"
`;
exports[`EuiContextMenuPanel updating items and content updates to items should not re-render if any items's watchedItemProps did not change 2`] = `
"<EuiContextMenuPanel items={{...}} hasFocus={true}>
<div className=\\"euiContextMenuPanel\\" onKeyDown={[Function]} tabIndex=\\"0\\" onAnimationEnd={[Function]}>
<div>
<EuiContextMenuItem data-counter={0} buttonRef={[Function: bound ]}>
<button className=\\"euiContextMenuItem\\" type=\\"button\\" disabled={[undefined]} data-counter={0}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option A
</span>
</span>
</button>
</EuiContextMenuItem>
<EuiContextMenuItem data-counter={1} buttonRef={[Function: bound ]}>
<button className=\\"euiContextMenuItem\\" type=\\"button\\" disabled={[undefined]} data-counter={1}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option B
</span>
</span>
</button>
</EuiContextMenuItem>
</div>
</div>
</EuiContextMenuPanel>"
`;
exports[`EuiContextMenuPanel updating items and content updates to items should re-render at all times when children exists 1`] = `
"<EuiContextMenuPanel hasFocus={true} items={{...}}>
<div className=\\"euiContextMenuPanel\\" onKeyDown={[Function]} tabIndex=\\"0\\" onAnimationEnd={[Function]}>
<div>
Hello World
</div>
</div>
</EuiContextMenuPanel>"
`;
exports[`EuiContextMenuPanel updating items and content updates to items should re-render at all times when children exists 2`] = `
"<EuiContextMenuPanel hasFocus={true} items={{...}}>
<div className=\\"euiContextMenuPanel\\" onKeyDown={[Function]} tabIndex=\\"0\\" onAnimationEnd={[Function]}>
<div>
More Salutations
</div>
</div>
</EuiContextMenuPanel>"
`;
exports[`EuiContextMenuPanel updating items and content updates to items should re-render if any items's watchedItemProps did change 1`] = `
"<EuiContextMenuPanel watchedItemProps={{...}} items={{...}} hasFocus={true}>
<div className=\\"euiContextMenuPanel\\" onKeyDown={[Function]} tabIndex=\\"0\\" onAnimationEnd={[Function]}>
<div>
<EuiContextMenuItem data-counter={0} buttonRef={[Function: bound ]}>
<button className=\\"euiContextMenuItem\\" type=\\"button\\" disabled={[undefined]} data-counter={0}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option A
</span>
</span>
</button>
</EuiContextMenuItem>
<EuiContextMenuItem data-counter={1} buttonRef={[Function: bound ]}>
<button className=\\"euiContextMenuItem\\" type=\\"button\\" disabled={[undefined]} data-counter={1}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option B
</span>
</span>
</button>
</EuiContextMenuItem>
</div>
</div>
</EuiContextMenuPanel>"
`;
exports[`EuiContextMenuPanel updating items and content updates to items should re-render if any items's watchedItemProps did change 2`] = `
"<EuiContextMenuPanel watchedItemProps={{...}} items={{...}} hasFocus={true}>
<div className=\\"euiContextMenuPanel\\" onKeyDown={[Function]} tabIndex=\\"0\\" onAnimationEnd={[Function]}>
<div>
<EuiContextMenuItem data-counter={2} buttonRef={[Function: bound ]}>
<button className=\\"euiContextMenuItem\\" type=\\"button\\" disabled={[undefined]} data-counter={2}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option A
</span>
</span>
</button>
</EuiContextMenuItem>
<EuiContextMenuItem data-counter={3} buttonRef={[Function: bound ]}>
<button className=\\"euiContextMenuItem\\" type=\\"button\\" disabled={[undefined]} data-counter={3}>
<span className=\\"euiContextMenu__itemLayout\\">
<span className=\\"euiContextMenuItem__text\\">
Option B
</span>
</span>
</button>
</EuiContextMenuItem>
</div>
</div>
</EuiContextMenuPanel>"
`;
17 changes: 15 additions & 2 deletions src/components/context_menu/context_menu_panel.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,21 @@ export class EuiContextMenuPanel extends Component {
return true;
}

// Check if any watched item properties changed by quick string comparison
if(this.getWatchedPropsForItems(nextProps.items) !== this.getWatchedPropsForItems(this.props.items)) {
// **
// this component should have either items or children,
// if there are items we can determine via `watchedItemProps` if we should update
// if there are children we can't know if they have changed so return true
// **

if (this.props.items != null) {
// Check if any watched item properties changed by quick string comparison
if(this.getWatchedPropsForItems(nextProps.items) !== this.getWatchedPropsForItems(this.props.items)) {
return true;
}
}

// it's not possible (in any good way) to know if `children` has changed, assume they might have
if (this.props.children != null) {
return true;
}

Expand Down
81 changes: 81 additions & 0 deletions src/components/context_menu/context_menu_panel.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,4 +364,85 @@ describe('EuiContextMenuPanel', () => {
});
});
});

describe('updating items and content', () => {
describe('updates to items', () => {
it(`should not re-render if any items's watchedItemProps did not change`, () => {
expect.assertions(2); // make sure the assertion in the `setProps` callback is executed

// by not passing `watchedItemProps` no changes to items should cause a re-render
const component = mount(
<EuiContextMenuPanel
items={[
<EuiContextMenuItem key="A" data-counter={0}>Option A</EuiContextMenuItem>,
<EuiContextMenuItem key="B" data-counter={1}>Option B</EuiContextMenuItem>,
]}
/>
);

expect(component.debug()).toMatchSnapshot();

component.setProps(
{
items: [
<EuiContextMenuItem key="A" data-counter={2}>Option A</EuiContextMenuItem>,
<EuiContextMenuItem key="B" data-counter={3}>Option B</EuiContextMenuItem>
]
},
() => {
expect(component.debug()).toMatchSnapshot();
}
);
});

it(`should re-render if any items's watchedItemProps did change`, () => {
expect.assertions(2); // make sure the assertion in the `setProps` callback is executed

// by referencing the `data-counter` property in `watchedItemProps`
// changes to the items should be picked up and re-rendered
const component = mount(
<EuiContextMenuPanel
watchedItemProps={['data-counter']}
items={[
<EuiContextMenuItem key="A" data-counter={0}>Option A</EuiContextMenuItem>,
<EuiContextMenuItem key="B" data-counter={1}>Option B</EuiContextMenuItem>,
]}
/>
);

expect(component.debug()).toMatchSnapshot();

component.setProps(
{
items: [
<EuiContextMenuItem key="A" data-counter={2}>Option A</EuiContextMenuItem>,
<EuiContextMenuItem key="B" data-counter={3}>Option B</EuiContextMenuItem>,
]
},
() => {
expect(component.debug()).toMatchSnapshot();
}
);
});

it(`should re-render at all times when children exists`, () => {
expect.assertions(2); // make sure the assertion in the `setProps` callback is executed

const component = mount(
<EuiContextMenuPanel>
Hello World
</EuiContextMenuPanel>
);

expect(component.debug()).toMatchSnapshot();

component.setProps(
{ children: 'More Salutations' },
() => {
expect(component.debug()).toMatchSnapshot();
}
);
});
});
});
});

0 comments on commit d5178c7

Please sign in to comment.