Skip to content

Commit

Permalink
Address review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
pugnascotia committed Apr 4, 2018
1 parent d9abcef commit 56cb55b
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 35 deletions.
5 changes: 1 addition & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,12 @@
**Bug fixes**

- Allow accordions to dynamically change height, and support values on radio inputs ([#613](https://github.com/elastic/eui/pull/613))
- Accordion toggle layout is no longer flagged responsive, in order to prevent unwanted stacking on mobile ([#613](https://github.com/elastic/eui/pull/613))

**Breaking changes**

- Support values on radio inputs. This is breaking because now the second argument to the radio `onChange` callback is the value, which bumps the change event to the third argument ([#613](https://github.com/elastic/eui/pull/613))

**Breaking changes**

- `EuiSelect` can pass any node as a value rather than just a string ([603](https://github.com/elastic/eui/pull/603))

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

- Modified drop shadow intensities and color. ([#607](https://github.com/elastic/eui/pull/607))
Expand Down
7 changes: 3 additions & 4 deletions src-docs/src/views/accordion/accordion_grow.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ class AccordionGrow extends Component {
}

render() {

const rows = []
const rows = [];
for (let i = 1; i <= this.state.counter; i++) {
rows.push(<p key={i}>Row {i}</p>);
}
Expand All @@ -42,13 +41,13 @@ class AccordionGrow extends Component {
onIncrease() {
this.setState(prevState => ({
counter: prevState.counter + 1
}))
}));
}

onDecrease() {
this.setState(prevState => ({
counter: Math.max(0, prevState.counter - 1)
}))
}));
}
}

Expand Down
18 changes: 9 additions & 9 deletions src/components/accordion/__snapshots__/accordion.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ exports[`EuiAccordion behavior closes when clicked twice 1`] = `
component="div"
gutterSize="s"
justifyContent="flexStart"
responsive={true}
responsive={false}
wrap={false}
>
<div
className="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter euiFlexGroup--responsive"
className="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter"
>
<EuiFlexItem
component="div"
Expand Down Expand Up @@ -159,11 +159,11 @@ exports[`EuiAccordion behavior opens when clicked once 1`] = `
component="div"
gutterSize="s"
justifyContent="flexStart"
responsive={true}
responsive={false}
wrap={false}
>
<div
className="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter euiFlexGroup--responsive"
className="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter"
>
<EuiFlexItem
component="div"
Expand Down Expand Up @@ -261,7 +261,7 @@ exports[`EuiAccordion is rendered 1`] = `
class="euiAccordion__button"
>
<div
class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter euiFlexGroup--responsive"
class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter"
>
<div
class="euiFlexItem euiFlexItem--flexGrowZero"
Expand Down Expand Up @@ -319,7 +319,7 @@ exports[`EuiAccordion props buttonContent is rendered 1`] = `
class="euiAccordion__button"
>
<div
class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter euiFlexGroup--responsive"
class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter"
>
<div
class="euiFlexItem euiFlexItem--flexGrowZero"
Expand Down Expand Up @@ -381,7 +381,7 @@ exports[`EuiAccordion props buttonContentClassName is rendered 1`] = `
class="euiAccordion__button"
>
<div
class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter euiFlexGroup--responsive"
class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter"
>
<div
class="euiFlexItem euiFlexItem--flexGrowZero"
Expand Down Expand Up @@ -439,7 +439,7 @@ exports[`EuiAccordion props extraAction is rendered 1`] = `
class="euiAccordion__button"
>
<div
class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter euiFlexGroup--responsive"
class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter"
>
<div
class="euiFlexItem euiFlexItem--flexGrowZero"
Expand Down Expand Up @@ -504,7 +504,7 @@ exports[`EuiAccordion props initialIsOpen is rendered 1`] = `
class="euiAccordion__button"
>
<div
class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter euiFlexGroup--responsive"
class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter"
>
<div
class="euiFlexItem euiFlexItem--flexGrowZero"
Expand Down
4 changes: 2 additions & 2 deletions src/components/accordion/accordion.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class EuiAccordion extends Component {
}

componentDidUpdate() {
const height = this.state.isOpen ? this.childContent.clientHeight: 0
const height = this.state.isOpen ? this.childContent.clientHeight: 0;

this.childWrapper.setAttribute('style', `height: ${height}px`);
}
Expand Down Expand Up @@ -94,7 +94,7 @@ export class EuiAccordion extends Component {
onClick={this.onToggle}
className={buttonClasses}
>
<EuiFlexGroup gutterSize="s" alignItems="center">
<EuiFlexGroup gutterSize="s" alignItems="center" responsive={false}>
<EuiFlexItem grow={false}>
{icon}
</EuiFlexItem>
Expand Down
26 changes: 10 additions & 16 deletions src/components/form/radio/radio_group.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe('EuiRadioGroup', () => {

describe('callbacks', () => {
test('id is used in callbacks when no value is available', () => {
const callback = jest.fn()
const callback = jest.fn();

const component = mount(
<EuiRadioGroup
Expand All @@ -96,18 +96,15 @@ describe('EuiRadioGroup', () => {
/>
);

component.find('input[id="2"]').simulate('change')
component.find('input[id="2"]').simulate('change');

expect(callback.mock.calls.length).toEqual(1);
expect(callback.mock.calls[0].length).toEqual(3);

expect(callback.mock.calls[0][0]).toEqual('2');
expect(callback.mock.calls[0][1]).toBeUndefined();
// The callback is also invoked with the event, but that's not assert-able.
expect(callback).toHaveBeenCalledTimes(1);
const event = expect.any(Object);
expect(callback).toHaveBeenCalledWith('2', undefined, event);
});

test('value is used in callbacks when available', () => {
const callback = jest.fn()
const callback = jest.fn();

const component = mount(
<EuiRadioGroup
Expand All @@ -120,14 +117,11 @@ describe('EuiRadioGroup', () => {
/>
);

component.find('input[id="2"]').simulate('change')

expect(callback.mock.calls.length).toEqual(1);
expect(callback.mock.calls[0].length).toEqual(3);
component.find('input[id="2"]').simulate('change');

expect(callback.mock.calls[0][0]).toEqual('2');
expect(callback.mock.calls[0][1]).toEqual('Value #2');
// The callback is also invoked with the event, but that's not assert-able.
expect(callback).toHaveBeenCalledTimes(1);
const event = expect.any(Object);
expect(callback).toHaveBeenCalledWith('2', 'Value #2', event);
});
});
});

0 comments on commit 56cb55b

Please sign in to comment.