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

Adds paddingSize prop to EuiAccordion #701

Merged
merged 4 commits into from
Apr 25, 2018
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
- Added support for text alignment with `EuiTextAlign` ([#683](https://github.com/elastic/eui/pull/683))
- `EuiBasicTable` added the `compressed` prop to allow for tables with smaller fonts and padding. ([#687](https://github.com/elastic/eui/pull/687))

**Bug fixes**

- Added a `paddingSize` prop to `EuiAccordion` to better mitigate situations where a nested `EuiFlexGroup` causes scrollbars ([#701](https://github.com/elastic/eui/pull/701))

**Breaking changes**

- Added responsive support for tables. This isn't technically a breaking change, but you will need to apply some new props (`hasActions`, `isSelectable`) for certain tables to make them look their best in mobile. **Responsive table views are on by default.** ([#584](https://github.com/elastic/eui/pull/584))
Expand Down
3 changes: 2 additions & 1 deletion src-docs/src/views/accordion/accordion.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ export default () => (

<EuiAccordion
id="accordion2"
buttonContent="You can click me as well"
buttonContent="An accordion with some padding applied through props"
paddingSize="l"
>
<EuiText>
<p>The content inside can be of any height.</p>
Expand Down
21 changes: 20 additions & 1 deletion src-docs/src/views/accordion/accordion_example.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { Fragment } from 'react';

import { renderToHtml } from '../../services';

Expand All @@ -9,6 +9,8 @@ import {
import {
EuiAccordion,
EuiCode,
EuiCallOut,
EuiSpacer,
} from '../../../../src/components';

import Accordion from './accordion';
Expand All @@ -33,6 +35,23 @@ const accordionGrowHtml = renderToHtml(AccordionGrow);

export const AccordionExample = {
title: 'Accordion',
intro: (
<Fragment>
<EuiCallOut
title="Take care including flex group content within accordions"
>
<p>
<EuiCode>EuiFlexGroup</EuiCode>&apos;s negative margins can sometimes
create scrollbars within <EuiCode>EuiAccordion</EuiCode> because of
the overflow tricks the used to hide content. If you run into this issue make
sure your <EuiCode>paddingSize</EuiCode> prop is large enough to account for
the <EuiCode>gutterSize</EuiCode> of any nested flex groups.
</p>
</EuiCallOut>

<EuiSpacer size="l" />
</Fragment>
),
sections: [{
title: 'Unstyled',
source: [{
Expand Down
1 change: 1 addition & 0 deletions src-docs/src/views/accordion/accordion_extra.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export default () => (
id="accordionExtra"
buttonContent="Click to open"
extraAction={<EuiButton size="s">Extra action!</EuiButton>}
paddingSize="l"
>
<div>Opened content.</div>
</EuiAccordion>
Expand Down
10 changes: 4 additions & 6 deletions src-docs/src/views/accordion/accordion_form.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,9 @@ export default () => (
buttonClassName="euiAccordionForm__button"
buttonContent={buttonContent}
extraAction={extraAction}
paddingSize="l"
>
<div className="euiAccordionForm__children">
{repeatableForm}
</div>
{repeatableForm}
</EuiAccordion>

<EuiAccordion
Expand All @@ -100,10 +99,9 @@ export default () => (
buttonClassName="euiAccordionForm__button"
buttonContent={buttonContent}
extraAction={extraAction}
paddingSize="l"
>
<div className="euiAccordionForm__children">
{repeatableForm}
</div>
{repeatableForm}
</EuiAccordion>
</div>
);
1 change: 1 addition & 0 deletions src-docs/src/views/accordion/accordion_grow.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class AccordionGrow extends Component {
id="accordion1"
buttonContent="Click me to toggle close / open"
initialIsOpen={true}
paddingSize="l"
>
<EuiText>
<EuiSpacer size='s' />
Expand Down
1 change: 1 addition & 0 deletions src-docs/src/views/accordion/accordion_open.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export default () => (
id="accordion1"
buttonContent="I am opened by default. Click me to toggle close / open"
initialIsOpen={true}
paddingSize="l"
>
<EuiText>
<p>Any content inside of <EuiCode>EuiAccordion</EuiCode> will appear here.</p>
Expand Down
48 changes: 39 additions & 9 deletions src/components/accordion/__snapshots__/accordion.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ exports[`EuiAccordion behavior closes when clicked twice 1`] = `
<EuiAccordion
id="6"
initialIsOpen={false}
paddingSize="none"
>
<div
className="euiAccordion"
Expand Down Expand Up @@ -116,7 +117,11 @@ exports[`EuiAccordion behavior closes when clicked twice 1`] = `
className="euiAccordion__childWrapper"
id="6"
>
<div />
<div>
<div
className=""
/>
</div>
</div>
</div>
</EuiAccordion>
Expand All @@ -126,6 +131,7 @@ exports[`EuiAccordion behavior opens when clicked once 1`] = `
<EuiAccordion
id="5"
initialIsOpen={false}
paddingSize="none"
>
<div
className="euiAccordion euiAccordion-isOpen"
Expand Down Expand Up @@ -237,7 +243,11 @@ exports[`EuiAccordion behavior opens when clicked once 1`] = `
className="euiAccordion__childWrapper"
id="5"
>
<div />
<div>
<div
className=""
/>
</div>
</div>
</div>
</EuiAccordion>
Expand Down Expand Up @@ -298,7 +308,11 @@ exports[`EuiAccordion is rendered 1`] = `
class="euiAccordion__childWrapper"
id="0"
>
<div />
<div>
<div
class=""
/>
</div>
</div>
</div>
`;
Expand Down Expand Up @@ -360,7 +374,11 @@ exports[`EuiAccordion props buttonContent is rendered 1`] = `
class="euiAccordion__childWrapper"
id="2"
>
<div />
<div>
<div
class=""
/>
</div>
</div>
</div>
`;
Expand Down Expand Up @@ -418,7 +436,11 @@ exports[`EuiAccordion props buttonContentClassName is rendered 1`] = `
class="euiAccordion__childWrapper"
id="1"
>
<div />
<div>
<div
class=""
/>
</div>
</div>
</div>
`;
Expand Down Expand Up @@ -483,7 +505,11 @@ exports[`EuiAccordion props extraAction is rendered 1`] = `
class="euiAccordion__childWrapper"
id="3"
>
<div />
<div>
<div
class=""
/>
</div>
</div>
</div>
`;
Expand Down Expand Up @@ -541,9 +567,13 @@ exports[`EuiAccordion props initialIsOpen is rendered 1`] = `
id="4"
>
<div>
<p>
You can see me.
</p>
<div
class=""
>
<p>
You can see me.
</p>
</div>
</div>
</div>
</div>
Expand Down
16 changes: 16 additions & 0 deletions src/components/accordion/_accordion.scss
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,22 @@
;
}

$paddingSizes: (
xs: $euiSizeXS,
s: $euiSizeS,
m: $euiSize,
l: $euiSizeL,
xl: $euiSizeXL,
);

// Create button modifiders based upon the map.
@each $name, $size in $paddingSizes {

.euiAccordion__padding--#{$name} {
padding: $size;
}
}

.euiAccordion.euiAccordion-isOpen {
.euiAccordion__childWrapper {
visibility: visible;
Expand Down
4 changes: 0 additions & 4 deletions src/components/accordion/_accordion_form.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
.euiAccordionForm__children {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was never properly componentized. I was using it in the example docs as a singleton class, which is why the forms with flex groups weren't breaking. It is no longer needed now that there's a prop to manage padding.

padding: $euiSizeL;
}

.euiAccordionForm__extraAction {
opacity: 0;
transition: opacity $euiAnimSpeedNormal $euiAnimSlightResistance;
Expand Down
46 changes: 44 additions & 2 deletions src/components/accordion/accordion.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@ import {
EuiFlexItem,
} from '../flex';

const paddingSizeToClassNameMap = {
none: null,
xs: 'euiAccordion__padding--xs',
s: 'euiAccordion__padding--s',
m: 'euiAccordion__padding--m',
l: 'euiAccordion__padding--l',
xl: 'euiAccordion__padding--xl',
};

export const PADDING_SIZES = Object.keys(paddingSizeToClassNameMap);

export class EuiAccordion extends Component {
constructor(props) {
super(props);
Expand Down Expand Up @@ -45,16 +56,22 @@ export class EuiAccordion extends Component {
buttonClassName,
buttonContentClassName,
extraAction,
paddingSize,
initialIsOpen, // eslint-disable-line no-unused-vars
...rest
} = this.props;


const classes = classNames(
'euiAccordion',
{
'euiAccordion-isOpen': this.state.isOpen,
},
className
className,
);

const paddingClass = classNames(
paddingSizeToClassNameMap[paddingSize],
);

const buttonClasses = classNames(
Expand Down Expand Up @@ -115,7 +132,9 @@ export class EuiAccordion extends Component {
id={id}
>
<div ref={node => { this.childContent = node; }}>
{children}
<div className={paddingClass}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to put the paddingClass directly on the ref'd div versus creating another dom node layer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that originally. It didn't work with the height calculations and messed up the calculation.

{children}
</div>
</div>
</div>
</div>
Expand All @@ -124,15 +143,38 @@ export class EuiAccordion extends Component {
}

EuiAccordion.propTypes = {
/**
* The content of the exposed accordion.
*/
children: PropTypes.node,
id: PropTypes.string.isRequired,
/**
* Class that will apply to the entire accordion.
*/
className: PropTypes.string,
/**
* Class that will apply to the trigger for the accordion.
*/
buttonContentClassName: PropTypes.string,
/**
* The content of the clickable trigger
*/
buttonContent: PropTypes.node,
/**
* Will appear right aligned against the button. Useful for separate actions like deletions.
*/
extraAction: PropTypes.node,
/**
* The accordion will start in the open state.
*/
initialIsOpen: PropTypes.bool,
/**
* The padding around the exposed accordion content.
*/
paddingSize: PropTypes.oneOf(PADDING_SIZES),
};

EuiAccordion.defaultProps = {
initialIsOpen: false,
paddingSize: 'none',
};