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 bug with EuiFlyout maxWidth interfering with responsive mode. #1124

Merged
merged 4 commits into from
Aug 17, 2018
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -3,6 +3,10 @@
- Added `zIndexAdjustment` to `EuiPopover` which allows tweaking the popover content's `z-index` ([#1097](https://github.com/elastic/eui/pull/1097))
- Added new `EuiSuperSelect` component and `hasArrow` prop to `EuiPopover` ([#921](https://github.com/elastic/eui/pull/921))

**Bug fixes**

- `EuiFlyout` responsive mode now gracefully overrides a custom `maxWidth` ([#1124](https://github.com/elastic/eui/pull/1124)

## [`3.6.1`](https://github.com/elastic/eui/tree/v3.6.1)

- Added TypeScript definition for `findTestSubject` test util ([#1106](https://github.com/elastic/eui/pull/1106))
Expand Down
8 changes: 4 additions & 4 deletions src-docs/src/views/flyout/flyout.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class Flyout extends Component {
const htmlCode = `<EuiFlyout ...>
<EuiFlyoutHeader hasBorder>
<EuiTitle size="m">
<h1></h1>
<h2></h2>
</EuiTitle>
</EuiFlyoutHeader>
<EuiFlyoutBody>
Expand All @@ -62,9 +62,9 @@ export class Flyout extends Component {
>
<EuiFlyoutHeader hasBorder>
<EuiTitle size="m">
<h1 id="flyoutTitle">
<h2 id="flyoutTitle">
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 updated these examples to use h2 because in most cases I've seen this heading won't make sense as a top-level heading; instead it will make more sense as a child of the top-level.

A typical flyout
</h1>
</h2>
</EuiTitle>
</EuiFlyoutHeader>
<EuiFlyoutBody>
Expand All @@ -85,7 +85,7 @@ export class Flyout extends Component {
return (
<div>
<EuiButton onClick={this.showFlyout}>
Show Flyout
Show flyout
</EuiButton>

{flyout}
Expand Down
6 changes: 3 additions & 3 deletions src-docs/src/views/flyout/flyout_complicated.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@ export class FlyoutComplicated extends Component {
>
<EuiFlyoutHeader hasBorder>
<EuiTitle size="m">
<h1 id="flyoutComplicatedTitle">
<h2 id="flyoutComplicatedTitle">
Flyout header
</h1>
</h2>
</EuiTitle>
<EuiSpacer size="s" />
<EuiText color="subdued">
Expand Down Expand Up @@ -212,7 +212,7 @@ export class FlyoutComplicated extends Component {
return (
<div>
<EuiButton onClick={this.showFlyout}>
Show Flyout
Show flyout
</EuiButton>

{flyout}
Expand Down
61 changes: 54 additions & 7 deletions src-docs/src/views/flyout/flyout_example.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,17 @@ import { FlyoutComplicated } from './flyout_complicated';
const flyoutComplicatedSource = require('!!raw-loader!./flyout_complicated');
const flyoutComplicatedHtml = renderToHtml(FlyoutComplicated);

import { FlyoutSize } from './flyout_size';
const flyoutSizeSource = require('!!raw-loader!./flyout_size');
const flyoutSizeHtml = renderToHtml(FlyoutSize);
import { FlyoutSmall } from './flyout_small';
const flyoutSmallSource = require('!!raw-loader!./flyout_small');
const flyoutSmallHtml = renderToHtml(FlyoutSmall);

import { FlyoutLarge } from './flyout_large';
const flyoutLargeSource = require('!!raw-loader!./flyout_large');
const flyoutLargeHtml = renderToHtml(FlyoutLarge);

import { FlyoutMaxWidth } from './flyout_max_width';
const flyoutMaxWidthSource = require('!!raw-loader!./flyout_max_width');
const flyoutMaxWidthHtml = renderToHtml(FlyoutMaxWidth);

export const FlyoutExample = {
title: 'Flyout',
Expand Down Expand Up @@ -89,15 +97,15 @@ export const FlyoutExample = {
demo: <FlyoutComplicated />,
},
{
title: 'Flyout sizing and focus',
title: 'Small flyout, ownFocus',
source: [
{
type: GuideSectionTypes.JS,
code: flyoutSizeSource,
code: flyoutSmallSource,
},
{
type: GuideSectionTypes.HTML,
code: flyoutSizeHtml,
code: flyoutSmallHtml,
},
],
text: (
Expand All @@ -107,7 +115,46 @@ export const FlyoutExample = {
also adds background overlay to reinforce your boundries.
</p>
),
demo: <FlyoutSize />,
demo: <FlyoutSmall />,
},
{
title: 'Large flyout',
source: [
{
type: GuideSectionTypes.JS,
code: flyoutLargeSource,
},
{
type: GuideSectionTypes.HTML,
code: flyoutLargeHtml,
},
],
text: (
<p>
In this example, we set <EuiCode>size</EuiCode> to <EuiCode>l</EuiCode>.
</p>
),
demo: <FlyoutLarge />,
},
{
title: 'maxWidth',
source: [
{
type: GuideSectionTypes.JS,
code: flyoutMaxWidthSource,
},
{
type: GuideSectionTypes.HTML,
code: flyoutMaxWidthHtml,
},
],
text: (
<p>
In this example, we set <EuiCode>maxWidth</EuiCode> to <EuiCode>448px</EuiCode>, to
set the width of the flyout at the ideal width for a form.
</p>
),
demo: <FlyoutMaxWidth />,
},
],
};
78 changes: 78 additions & 0 deletions src-docs/src/views/flyout/flyout_large.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import React, {
Component,
} from 'react';

import {
EuiFlyout,
EuiFlyoutHeader,
EuiFlyoutBody,
EuiButton,
EuiText,
EuiTitle,
} from '../../../../src/components';

export class FlyoutLarge extends Component {
constructor(props) {
super(props);

this.state = {
isFlyoutVisible: false,
isSwitchChecked: true,
};

this.closeFlyout = this.closeFlyout.bind(this);
this.showFlyout = this.showFlyout.bind(this);
}

onSwitchChange = () => {
this.setState({
isSwitchChecked: !this.state.isSwitchChecked,
});
}

closeFlyout() {
this.setState({ isFlyoutVisible: false });
}

showFlyout() {
this.setState({ isFlyoutVisible: true });
}

render() {

let flyout;
if (this.state.isFlyoutVisible) {
flyout = (
<EuiFlyout
onClose={this.closeFlyout}
size="l"
aria-labelledby="flyoutLargeTitle"
>
<EuiFlyoutHeader hasBorder>
<EuiTitle size="m">
<h2 id="flyoutLargeTitle">
A large flyout
</h2>
</EuiTitle>
</EuiFlyoutHeader>
<EuiFlyoutBody>
<EuiText>
<p>
The large flyout is very wide.
</p>
</EuiText>
</EuiFlyoutBody>
</EuiFlyout>
);
}
return (
<div>
<EuiButton onClick={this.showFlyout}>
Show large flyout
</EuiButton>

{flyout}
</div>
);
}
}
128 changes: 128 additions & 0 deletions src-docs/src/views/flyout/flyout_max_width.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
import React, {
Component,
} from 'react';

import {
EuiFlyout,
EuiFlyoutBody,
EuiFlyoutHeader,
EuiButton,
EuiText,
EuiTitle,
EuiFieldText,
EuiForm,
EuiFormRow,
EuiFilePicker,
EuiRange,
EuiSelect,
EuiSpacer,
} from '../../../../src/components';

export class FlyoutMaxWidth extends Component {
constructor(props) {
super(props);

this.state = {
isFlyoutVisible: false,
isSwitchChecked: true,
};

this.closeFlyout = this.closeFlyout.bind(this);
this.showFlyout = this.showFlyout.bind(this);
}

onSwitchChange = () => {
this.setState({
isSwitchChecked: !this.state.isSwitchChecked,
});
}

closeFlyout() {
this.setState({ isFlyoutVisible: false });
}

showFlyout() {
this.setState({ isFlyoutVisible: true });
}

render() {
let flyout;

if (this.state.isFlyoutVisible) {
flyout = (
<EuiFlyout
onClose={this.closeFlyout}
aria-labelledby="flyoutMaxWidthTitle"
maxWidth="448px"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we change the examples to not pass the 'px'? I intentionally wanted to mimic the way React handles the styles prop where if you send a number maxWidth={448}, it would treat it as a px value. But if you send a string maxWidth="40rem" it would apply it as is. I was thinking this would be an easier way for consumers to pass values without having to remember to append 'px'.

>
<EuiFlyoutHeader hasBorder>
<EuiTitle size="m">
<h2 id="flyoutMaxWidthTitle">
448px wide flyout
</h2>
</EuiTitle>
</EuiFlyoutHeader>
<EuiFlyoutBody>
<EuiText>
<p>
In many cases, you&rsquo;ll want to set a custom width that&rdquo;s tailored to your content.
In this case, the flyout is an ideal width for form elements.
</p>
</EuiText>

<EuiSpacer />

<EuiForm>
<EuiFormRow
label="Text field"
helpText="I am some friendly help text."
>
<EuiFieldText name="first" />
</EuiFormRow>

<EuiFormRow
label="Select (with no initial selection)"
>
<EuiSelect
hasNoInitialSelection
options={[
{ value: 'option_one', text: 'Option one' },
{ value: 'option_two', text: 'Option two' },
{ value: 'option_three', text: 'Option three' },
]}
/>
</EuiFormRow>

<EuiFormRow
label="File picker"
>
<EuiFilePicker />
</EuiFormRow>

<EuiFormRow
label="Range"
>
<EuiRange
min={0}
max={100}
name="range"
id="range"
/>
</EuiFormRow>
</EuiForm>
</EuiFlyoutBody>
</EuiFlyout>
);
}

return (
<div>
<EuiButton onClick={this.showFlyout}>
Show max-width flyout
</EuiButton>

{flyout}
</div>
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
EuiTitle,
} from '../../../../src/components';

export class FlyoutSize extends Component {
export class FlyoutSmall extends Component {
constructor(props) {
super(props);

Expand Down Expand Up @@ -47,13 +47,13 @@ export class FlyoutSize extends Component {
ownFocus
onClose={this.closeFlyout}
size="s"
aria-labelledby="flyoutSizeTitle"
aria-labelledby="flyoutSmallTitle"
>
<EuiFlyoutHeader hasBorder>
<EuiTitle size="s">
<h1 id="flyoutSizeTitle">
<h2 id="flyoutSmallTitle">
A small flyout
</h1>
</h2>
</EuiTitle>
</EuiFlyoutHeader>
<EuiFlyoutBody>
Expand All @@ -69,7 +69,7 @@ export class FlyoutSize extends Component {
return (
<div>
<EuiButton onClick={this.showFlyout}>
Show Flyout
Show small flyout
</EuiButton>

{flyout}
Expand Down
Loading