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

fix(panel): append panel body as a child element #3561

Merged
merged 6 commits into from
Mar 12, 2019
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
75 changes: 42 additions & 33 deletions src/components/Panel/Panel.js
Original file line number Diff line number Diff line change
@@ -1,44 +1,53 @@
import React from 'preact-compat';
import React, { Component } from 'preact-compat';
import PropTypes from 'prop-types';
import cx from 'classnames';
import Template from '../Template/Template';

const Panel = ({ cssClasses, hidden, templateProps, data, onRef }) => (
<div
className={cx(cssClasses.root, {
[cssClasses.noRefinementRoot]: hidden,
})}
hidden={hidden}
>
{templateProps.templates.header && (
<Template
{...templateProps}
templateKey="header"
rootProps={{
className: cssClasses.header,
}}
data={data}
/>
)}
class Panel extends Component {
componentDidMount() {
this.bodyRef.appendChild(this.props.bodyElement);
}

<div className={cssClasses.body} ref={onRef} />
render() {
const { cssClasses, hidden, templateProps, data } = this.props;

{templateProps.templates.footer && (
<Template
{...templateProps}
templateKey="footer"
rootProps={{
className: cssClasses.footer,
}}
data={data}
/>
)}
</div>
);
return (
<div
className={cx(cssClasses.root, {
[cssClasses.noRefinementRoot]: hidden,
})}
hidden={hidden}
>
{templateProps.templates.header && (
<Template
{...templateProps}
templateKey="header"
rootProps={{
className: cssClasses.header,
}}
data={data}
/>
)}

<div className={cssClasses.body} ref={node => (this.bodyRef = node)} />

{templateProps.templates.footer && (
<Template
{...templateProps}
templateKey="footer"
rootProps={{
className: cssClasses.footer,
}}
data={data}
/>
)}
</div>
);
}
}

Panel.propTypes = {
// Prop to get the panel body reference to insert the widget
onRef: PropTypes.func,
bodyElement: PropTypes.instanceOf(Element).isRequired,
cssClasses: PropTypes.shape({
root: PropTypes.string.isRequired,
noRefinementRoot: PropTypes.string.isRequired,
Expand Down
78 changes: 37 additions & 41 deletions src/components/Panel/__tests__/Panel-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,64 +2,60 @@ import React from 'react';
import { mount } from 'enzyme';
import Panel from '../Panel';

const cssClasses = {
root: 'root',
noRefinementRoot: 'noRefinementRoot',
body: 'body',
header: 'header',
footer: 'footer',
};

const getDefaultProps = () => ({
bodyElement: document.createElement('div'),
cssClasses,
hidden: false,
data: {},
templateProps: {
templates: {
header: 'Header',
footer: 'Footer',
},
},
});

describe('Panel', () => {
test('should render component with default props', () => {
const props = {
cssClasses: {
root: 'root',
noRefinementRoot: 'noRefinementRoot',
body: 'body',
header: 'header',
footer: 'footer',
},
hidden: false,
data: {},
templateProps: {
templates: {
header: 'Header',
footer: 'Footer',
},
},
...getDefaultProps(),
};

const wrapper = mount(<Panel {...props} />);

expect(wrapper.find('.root')).toHaveLength(1);
expect(wrapper.find('.noRefinementRoot')).toHaveLength(0);
expect(wrapper.find('.body')).toHaveLength(1);
expect(wrapper.find('.header')).toHaveLength(1);
expect(wrapper.find('.footer')).toHaveLength(1);
expect(wrapper.find('.header').text()).toBe('Header');
expect(wrapper.find('.footer').text()).toBe('Footer');
expect(wrapper.find(`.${cssClasses.root}`).exists()).toBe(true);
expect(wrapper.find(`.${cssClasses.noRefinementRoot}`).exists()).toBe(
false
);
expect(wrapper.find(`.${cssClasses.body}`).exists()).toBe(true);
expect(wrapper.find(`.${cssClasses.header}`).exists()).toBe(true);
expect(wrapper.find(`.${cssClasses.footer}`).exists()).toBe(true);
expect(wrapper.find(`.${cssClasses.header}`).text()).toBe('Header');
expect(wrapper.find(`.${cssClasses.footer}`).text()).toBe('Footer');
expect(wrapper).toMatchSnapshot();
});

test('should render component with `hidden` prop', () => {
const props = {
cssClasses: {
root: 'root',
noRefinementRoot: 'noRefinementRoot',
body: 'body',
header: 'header',
footer: 'footer',
},
...getDefaultProps(),
hidden: true,
data: {},
templateProps: {
templates: {
header: 'Header',
footer: 'Footer',
},
},
};

const wrapper = mount(<Panel {...props} />);

expect(wrapper.find('.root')).toHaveLength(1);
expect(wrapper.find('.noRefinementRoot')).toHaveLength(1);
expect(wrapper.find('.body')).toHaveLength(1);
expect(wrapper.find('.header')).toHaveLength(1);
expect(wrapper.find('.footer')).toHaveLength(1);
expect(wrapper.find(`.${cssClasses.root}`).exists()).toBe(true);
expect(wrapper.find(`.${cssClasses.noRefinementRoot}`).exists()).toBe(true);
expect(wrapper.find(`.${cssClasses.body}`).exists()).toBe(true);
expect(wrapper.find(`.${cssClasses.header}`).exists()).toBe(true);
expect(wrapper.find(`.${cssClasses.footer}`).exists()).toBe(true);
expect(wrapper.props().hidden).toBe(true);
expect(wrapper).toMatchSnapshot();
});
Expand Down
58 changes: 50 additions & 8 deletions src/widgets/panel/__tests__/panel-test.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,36 @@
import { render, unmountComponentAtNode } from 'preact-compat';
import panel from '../panel';

jest.mock('preact-compat', () => {
const module = require.requireActual('preact-compat');

module.render = jest.fn();
module.unmountComponentAtNode = jest.fn();

return module;
});

describe('Usage', () => {
test('without arguments does not throw', () => {
expect(() => panel()).not.toThrow();
expect(() => {
panel();
}).not.toThrow();
});

test('with templates does not throw', () => {
expect(() =>
expect(() => {
panel({
templates: { header: 'header' },
})
).not.toThrow();
});
}).not.toThrow();
});

test('with `hidden` as function does not throw', () => {
expect(() =>
expect(() => {
panel({
hidden: () => true,
})
).not.toThrow();
});
}).not.toThrow();
});

test('with `hidden` as boolean warns', () => {
Expand All @@ -34,10 +46,40 @@ describe('Usage', () => {
test('with a widget without `container` throws', () => {
const fakeWidget = () => {};

expect(() => panel()(fakeWidget)({})).toThrowErrorMatchingInlineSnapshot(`
expect(() => {
panel()(fakeWidget)({});
}).toThrowErrorMatchingInlineSnapshot(`
"The \`container\` option is required in the widget within the panel.

See documentation: https://www.algolia.com/doc/api-reference/widgets/panel/js/"
`);
});
});

describe('Lifecycle', () => {
beforeEach(() => {
render.mockClear();
unmountComponentAtNode.mockClear();
});

test('calls the inner widget lifecycle', () => {
const widget = {
init: jest.fn(),
render: jest.fn(),
dispose: jest.fn(),
};
const widgetFactory = () => widget;

const widgetWithPanel = panel()(widgetFactory)({
container: document.createElement('div'),
});

widgetWithPanel.init({});
widgetWithPanel.render({});
widgetWithPanel.dispose({});

expect(widget.init).toHaveBeenCalledTimes(1);
expect(widget.render).toHaveBeenCalledTimes(1);
expect(widget.dispose).toHaveBeenCalledTimes(1);
});
});
22 changes: 11 additions & 11 deletions src/widgets/panel/panel.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,22 @@ import Panel from '../../components/Panel/Panel';
const withUsage = createDocumentationMessageGenerator({ name: 'panel' });
const suit = component('Panel');

const renderer = ({ containerNode, cssClasses, templateProps }) => ({
options,
hidden,
}) => {
let bodyRef = null;

const renderer = ({
containerNode,
bodyContainerNode,
cssClasses,
templateProps,
}) => ({ options, hidden }) => {
render(
<Panel
cssClasses={cssClasses}
hidden={hidden}
templateProps={templateProps}
data={options}
onRef={ref => (bodyRef = ref)}
bodyElement={bodyContainerNode}
/>,
containerNode
);

return { bodyRef };
};

/**
Expand Down Expand Up @@ -85,6 +83,7 @@ export default function panel({
`The \`hidden\` option in the "panel" widget expects a function returning a boolean (received "${typeof hidden}" type).`
);

const bodyContainerNode = document.createElement('div');
const cssClasses = {
root: cx(suit(), userCssClasses.root),
noRefinementRoot: cx(
Expand Down Expand Up @@ -112,18 +111,19 @@ export default function panel({

const renderPanel = renderer({
Copy link
Contributor

@samouss samouss Mar 6, 2019

Choose a reason for hiding this comment

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

We can remove the indirection created by the renderer. We can avoid to return a new function from the renderer and provide the full list of argument each time call the function. We do this in widgets because we don't have the control on the render call (which can be avoided anyway). Here we do have the control (see below). We can do it in another PR.

const renderer = ({
  containerNode,
  bodyContainerNode,
  cssClasses,
  templateProps,
  options,
  hidden
}) => {
  render(
    <Panel
      cssClasses={cssClasses}
      hidden={hidden}
      templateProps={templateProps}
      data={options}
      bodyElement={bodyContainerNode}
    />,
    containerNode
  );
};

containerNode: getContainerNode(container),
bodyContainerNode,
cssClasses,
templateProps,
});

const { bodyRef } = renderPanel({
renderPanel({
options: {},
hidden: true,
});

const widget = widgetFactory({
...widgetOptions,
container: getContainerNode(bodyRef),
container: bodyContainerNode,
});

return {
Expand Down
21 changes: 12 additions & 9 deletions stories/panel.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,37 +21,40 @@ storiesOf('Panel', module)
})
)
.add(
'with ratingMenu',
'with range input',
Copy link
Member Author

Choose a reason for hiding this comment

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

I updated these stories because they conflicted with the playground.

withHits(({ search, container, instantsearch }) => {
search.addWidget(
instantsearch.widgets.panel({
templates: {
header: ({ results }) =>
`Header ${results ? `| ${results.nbHits} results` : ''}`,
header: 'Price',
footer: 'Footer',
},
hidden: ({ results }) => results.nbHits === 0,
})(instantsearch.widgets.ratingMenu)({
})(instantsearch.widgets.rangeInput)({
container,
attribute: 'price',
})
);
})
)
.add(
'with menu',
'with range slider',
withHits(({ search, container, instantsearch }) => {
search.addWidget(
instantsearch.widgets.panel({
templates: {
header: ({ results }) =>
`Header ${results ? `| ${results.nbHits} results` : ''}`,
header: 'Price',
footer: 'Footer',
},
hidden: ({ results }) => results.nbHits === 0,
})(instantsearch.widgets.menu)({
})(instantsearch.widgets.rangeSlider)({
container,
attribute: 'brand',
attribute: 'price',
tooltips: {
format(rawValue) {
return `$${Math.round(rawValue).toLocaleString()}`;
},
},
})
);
})
Expand Down