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

[Hidden] Change children type to allow many and add children tests #8082

Merged
merged 9 commits into from
Sep 8, 2017
13 changes: 4 additions & 9 deletions src/Hidden/HiddenCss.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import React from 'react';
import warning from 'warning';
import classNames from 'classnames';
import { keys as breakpoints } from '../styles/createBreakpoints';
import { capitalizeFirstLetter } from '../utils/helpers';
import withStyles from '../styles/withStyles';
Expand Down Expand Up @@ -67,7 +66,9 @@ function HiddenCss(props: AllProps) {
warning(
Object.keys(other).length === 0 ||
(Object.keys(other).length === 1 && other.hasOwnProperty('ref')),
`Material-UI: unsupported properties received ${JSON.stringify(other)} by \`<Hidden />\`.`,
`Material-UI: unsupported properties received ${Object.keys(other).join(
', ',
)} by \`<Hidden />\`.`,
);

const className = [];
Expand All @@ -89,13 +90,7 @@ function HiddenCss(props: AllProps) {
className.push(classes[`only${capitalizeFirstLetter(only)}`]);
}

if (!React.isValidElement(children)) {
return null;
}

return React.cloneElement(children, {
className: classNames(children.props.className, className.join(' ')),
});
return <span className={className}>{children}</span>;
Copy link
Member

Choose a reason for hiding this comment

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

What do you think of adding a warning if ...other is non empty?

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'll alter the current warning, since a ref is no longer valid there.

}

export default withStyles(styles, { name: 'MuiHiddenCss' })(HiddenCss);
53 changes: 48 additions & 5 deletions src/Hidden/HiddenCss.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import { assert } from 'chai';
import { createShallow, getClasses } from '../test-utils';
import HiddenCss from './HiddenCss';

const Foo = () => <div>bar</div>;

describe('<HiddenCss />', () => {
let shallow;
let classes;

before(() => {
shallow = createShallow();
shallow = createShallow({ untilSelector: 'span' });
classes = getClasses(
<HiddenCss>
<div />
Expand All @@ -25,7 +27,11 @@ describe('<HiddenCss />', () => {
<div className="foo" />
</HiddenCss>,
);
assert.strictEqual(wrapper.props().className, `foo ${classes.onlySm}`);
assert.strictEqual(wrapper.type(), 'span');
assert.strictEqual(wrapper.hasClass(classes.onlySm), true);
const div = wrapper.childAt(0);
assert.strictEqual(div.type(), 'div');
assert.strictEqual(div.props().className, 'foo');
});

it('should be ok with mdDown', () => {
Expand All @@ -34,7 +40,7 @@ describe('<HiddenCss />', () => {
<div className="foo" />
</HiddenCss>,
);
assert.strictEqual(wrapper.props().className, `foo ${classes.mdDown}`);
assert.strictEqual(wrapper.hasClass(classes.mdDown), true);
});

it('should be ok with mdUp', () => {
Expand All @@ -43,13 +49,50 @@ describe('<HiddenCss />', () => {
<div className="foo" />
</HiddenCss>,
);
assert.strictEqual(wrapper.props().className, `foo ${classes.mdUp}`);
assert.strictEqual(wrapper.hasClass(classes.mdUp), true);
});
});

describe('prop: children', () => {
it('should work when empty', () => {
shallow(<HiddenCss mdUp />);
const wrapper = shallow(<HiddenCss mdUp />);
assert.strictEqual(wrapper.type(), 'span');
assert.strictEqual(wrapper.hasClass(classes.mdUp), true);
assert.isNull(wrapper.childAt(0).type());
});

it('should work when text Node', () => {
const wrapper = shallow(<HiddenCss mdUp>foo</HiddenCss>);
assert.strictEqual(wrapper.type(), 'span');
assert.strictEqual(wrapper.hasClass(classes.mdUp), true);
assert.strictEqual(wrapper.childAt(0).text(), 'foo');
});

it('should work when Element', () => {
const wrapper = shallow(
<HiddenCss mdUp>
<Foo />
</HiddenCss>,
);
assert.strictEqual(wrapper.type(), 'span');
assert.strictEqual(wrapper.hasClass(classes.mdUp), true);
assert.strictEqual(wrapper.childAt(0).is(Foo), true);
});

it('should work when mixed ChildrenArray', () => {
const wrapper = shallow(
<HiddenCss mdUp>
<Foo />
<Foo />
foo
</HiddenCss>,
);

assert.strictEqual(wrapper.type(), 'span');
assert.strictEqual(wrapper.hasClass(classes.mdUp), true);
assert.strictEqual(wrapper.childAt(0).is(Foo), true);
assert.strictEqual(wrapper.childAt(1).is(Foo), true);
assert.strictEqual(wrapper.childAt(2).text(), 'foo');
});
});
});
22 changes: 19 additions & 3 deletions src/Hidden/HiddenJs.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import HiddenJs from './HiddenJs';
import type { Breakpoint } from '../styles/createBreakpoints';
import Typography from '../Typography';

const Foo = () => <div>bar</div>;

describe('<HiddenJs />', () => {
let shallow;

Expand Down Expand Up @@ -41,12 +43,26 @@ describe('<HiddenJs />', () => {
it(descriptions[upDownOnly], () => {
const props = { width, [prop]: breakpoint };

// children
// Node
let wrapper = shallow(<HiddenJs {...props}>foo</HiddenJs>);
assert.isNull(wrapper.type(), 'should render null');

// element
wrapper = shallow(<HiddenJs {...props}>foo</HiddenJs>);
// Element
wrapper = shallow(
<HiddenJs {...props}>
<Foo />
</HiddenJs>,
);
assert.isNull(wrapper.type(), 'should render null');

// ChildrenArray
wrapper = shallow(
<HiddenJs {...props}>
<Foo />
<Foo />
foo
</HiddenJs>,
);
assert.isNull(wrapper.type(), 'should render null');
});
});
Expand Down
4 changes: 2 additions & 2 deletions src/Hidden/types.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
// @flow

import type { Element } from 'react';
import type { ChildrenArray, Node } from 'react';
import type { Breakpoint } from '../styles/createBreakpoints';

export type HiddenProps = {
/**
* The content of the component.
*/
children?: Element<*>,
children?: $ReadOnlyArray<ChildrenArray<Node>>,
/**
* @ignore
*/
Expand Down