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

Updating Popovers to use Panel Padding vars #229

Merged
merged 30 commits into from
Dec 21, 2017
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
0c4dcf0
working commit
cchaos Dec 19, 2017
9eb945f
Created a panel padding SASS map and using in popover to propagate down
cchaos Dec 20, 2017
93a4a0d
Removed border from title
cchaos Dec 20, 2017
70bd675
Reducing spacing
cchaos Dec 20, 2017
0131c65
Fixed a couple other examples using popover
cchaos Dec 20, 2017
ff4b42f
Fix button vertical alignment (#232)
snide Dec 20, 2017
841058d
Update CHANGELOG for 0.0.8.
cjcenizal Dec 20, 2017
655ae74
Updated documentation.
cjcenizal Dec 20, 2017
b4e45c4
0.0.8
cjcenizal Dec 20, 2017
c94c4c7
Give help / error text proper line-height (#234)
snide Dec 20, 2017
3f866df
Update release scripts to push to upstream instead of origin. (#233)
cjcenizal Dec 20, 2017
bb89192
Rename EuiFlexGroup classes to reflect CSS values, remove primary Tex…
cjcenizal Dec 20, 2017
46073f0
Update CHANGELOG for 0.0.9.
cjcenizal Dec 20, 2017
97dd332
Updated documentation.
cjcenizal Dec 20, 2017
b6f8e45
0.0.9
cjcenizal Dec 20, 2017
7df7069
Adding specific example for altering padding with title
cchaos Dec 21, 2017
38c7e7a
import panel file directly to make sure we grab the map
cchaos Dec 21, 2017
21b5568
Removing padding props from the title section
cchaos Dec 21, 2017
4a2938e
Last few edits
cchaos Dec 21, 2017
174bf8a
working commit
cchaos Dec 19, 2017
6afd194
Created a panel padding SASS map and using in popover to propagate down
cchaos Dec 20, 2017
2a2df38
Removed border from title
cchaos Dec 20, 2017
26a173f
Reducing spacing
cchaos Dec 20, 2017
9cc4c0e
Fixed a couple other examples using popover
cchaos Dec 20, 2017
a7af4d2
Adding specific example for altering padding with title
cchaos Dec 21, 2017
5047dd5
import panel file directly to make sure we grab the map
cchaos Dec 21, 2017
e933daa
Removing padding props from the title section
cchaos Dec 21, 2017
5c7d42f
Last few edits
cchaos Dec 21, 2017
93923a3
Merge branch 'popovers' of https://github.com/cchaos/eui into popovers
cchaos Dec 21, 2017
0536246
changelog
cchaos Dec 21, 2017
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
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ export class GuideThemeSelector extends Component {
isOpen={this.state.isThemePopoverOpen}
closePopover={this.closeThemePopover}
panelPaddingSize="none"
withTitle
anchorPosition="downRight"
>
<EuiContextMenuPanel
Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/form/inline_form_popover.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export default class extends Component {
isOpen={this.state.isPopoverOpen}
closePopover={this.closePopover.bind(this)}
>
<div style={{ width: 500, padding: 16 }}>
<div style={{ width: 500 }}>
{formSample}
</div>
</EuiPopover>
Expand Down
28 changes: 23 additions & 5 deletions src-docs/src/views/popover/popover_example.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ import PopoverWithTitle from './popover_with_title';
const popoverWithTitleSource = require('!!raw-loader!./popover_with_title');
const popoverWithTitleHtml = renderToHtml(PopoverWithTitle);

import PopoverWithTitlePadding from './popover_with_title_padding';
const popoverWithTitlePaddingSource = require('!!raw-loader!./popover_with_title_padding');
const popoverWithTitlePaddingHtml = renderToHtml(PopoverWithTitlePadding);

export const PopoverExample = {
title: 'Popover',
sections: [{
Expand Down Expand Up @@ -112,11 +116,6 @@ export const PopoverExample = {
<EuiCode>EuiPopoverTitle</EuiCode> nested somwhere in the child
prop.
</p>
<p>
Note that when using popover titles, you will need to
set <EuiCode>panelPaddingSize=&quot;none&quot;</EuiCode> and apply
some sort of padding around your content block itself.
</p>
</div>
),
demo: <PopoverWithTitle />,
Expand All @@ -137,5 +136,24 @@ export const PopoverExample = {
</p>
),
demo: <PopoverPanelClassName />,
}, {
title: 'Popover with title and padding size',
source: [{
type: GuideSectionTypes.JS,
code: popoverWithTitlePaddingSource,
}, {
type: GuideSectionTypes.HTML,
code: popoverWithTitlePaddingHtml,
}],
text: (
<div>
<p>
When using popover titles, you can still propogate the padding size
by using <EuiCode>panelPaddingSize</EuiCode>. This will only affect
the horizontal padding of the title and the overall padding of the content.
</p>
</div>
),
demo: <PopoverWithTitlePadding />,
}],
};
15 changes: 6 additions & 9 deletions src-docs/src/views/popover/popover_with_title.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,12 @@ export default class extends Component {
closePopover={this.closePopover1.bind(this)}
anchorPosition="downCenter"
withTitle
panelPaddingSize="none"
>
<EuiPopoverTitle>Hello, I&rsquo;m a popover title</EuiPopoverTitle>
<div style={{ width: '300px', padding: 16 }}>
<div style={{ width: '300px' }}>
<EuiText>
<p>
Popover content
Popover content with default padding
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also update the text of the buttons to also reflect that each one demonstrates a different padding size?

Copy link
Contributor

Choose a reason for hiding this comment

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

We're also missing an example of small padding, aren't we? Maybe we should just create a section dedicated to demonstrating the different padding options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'll just create a new section

</p>
</EuiText>
</div>
Expand All @@ -98,13 +97,12 @@ export default class extends Component {
closePopover={this.closePopover2.bind(this)}
anchorPosition="upCenter"
withTitle
panelPaddingSize="none"
>
<EuiPopoverTitle>Hello, I&rsquo;m a popover title</EuiPopoverTitle>
<div style={{ width: '300px', padding: 16 }}>
<div style={{ width: '300px' }}>
<EuiText>
<p>
Popover content
Popover content with large padding
</p>
</EuiText>
</div>
Expand All @@ -124,13 +122,12 @@ export default class extends Component {
closePopover={this.closePopover3.bind(this)}
anchorPosition="rightUp"
withTitle
panelPaddingSize="none"
>
<EuiPopoverTitle>Hello, I&rsquo;m a popover title</EuiPopoverTitle>
<div style={{ width: '300px', padding: 16 }}>
<div style={{ width: '300px' }}>
<EuiText>
<p>
Popover content
Popover content with no padding
</p>
</EuiText>
</div>
Expand Down
179 changes: 179 additions & 0 deletions src-docs/src/views/popover/popover_with_title_padding.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
import React, {
Component,
} from 'react';

import {
EuiPopover,
EuiPopoverTitle,
EuiButton,
EuiFlexGroup,
EuiFlexItem,
EuiText
} from '../../../../src/components';

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

this.state = {
isPopoverOpen: false,
};
}

onButtonClick1() {
this.setState({
isPopoverOpen1: !this.state.isPopoverOpen1,
});
}

closePopover1() {
this.setState({
isPopoverOpen1: false,
});
}

onButtonClick2() {
this.setState({
isPopoverOpen2: !this.state.isPopoverOpen2,
});
}

closePopover2() {
this.setState({
isPopoverOpen2: false,
});
}

onButtonClick3() {
this.setState({
isPopoverOpen3: !this.state.isPopoverOpen3,
});
}

closePopover3() {
this.setState({
isPopoverOpen3: false,
});
}

onButtonClick4() {
this.setState({
isPopoverOpen4: !this.state.isPopoverOpen4,
});
}

closePopover4() {
this.setState({
isPopoverOpen4: false,
});
}

render() {
return (
<EuiFlexGroup wrap={true}>
<EuiFlexItem grow={false}>
<EuiPopover
id="titleWithSmallPadding"
ownFocus
button={(
<EuiButton iconType="arrowDown" iconSide="right" onClick={this.onButtonClick2.bind(this)}>
Title and small padding
</EuiButton>
)}
isOpen={this.state.isPopoverOpen2}
closePopover={this.closePopover2.bind(this)}
anchorPosition="upCenter"
withTitle
panelPaddingSize="s"
>
<EuiPopoverTitle>Hello, I&rsquo;m a popover title</EuiPopoverTitle>
<div style={{ width: '300px' }}>
<EuiText>
<p>
Popover content
</p>
</EuiText>
</div>
</EuiPopover>
</EuiFlexItem>

<EuiFlexItem grow={false}>
<EuiPopover
id="titleWithDefaultPadding"
ownFocus
button={(
<EuiButton iconType="arrowDown" iconSide="right" onClick={this.onButtonClick1.bind(this)}>
Title and default padding (m)
</EuiButton>
)}
isOpen={this.state.isPopoverOpen1}
closePopover={this.closePopover1.bind(this)}
anchorPosition="upCenter"
withTitle
>
<EuiPopoverTitle>Hello, I&rsquo;m a popover title</EuiPopoverTitle>
<div style={{ width: '300px' }}>
<EuiText>
<p>
Popover content
</p>
</EuiText>
</div>
</EuiPopover>
</EuiFlexItem>

<EuiFlexItem grow={false}>
<EuiPopover
id="titleWithLargePadding"
ownFocus
button={(
<EuiButton iconType="arrowDown" iconSide="right" onClick={this.onButtonClick4.bind(this)}>
Title and large padding
</EuiButton>
)}
isOpen={this.state.isPopoverOpen4}
closePopover={this.closePopover4.bind(this)}
anchorPosition="upCenter"
withTitle
panelPaddingSize="l"
>
<EuiPopoverTitle>Hello, I&rsquo;m a popover title</EuiPopoverTitle>
<div style={{ width: '300px' }}>
<EuiText>
<p>
Popover content
</p>
</EuiText>
</div>
</EuiPopover>
</EuiFlexItem>

<EuiFlexItem grow={false}>
<EuiPopover
id="titleWithNoPadding"
ownFocus
button={(
<EuiButton iconType="arrowDown" iconSide="right" onClick={this.onButtonClick3.bind(this)}>
Title and no padding
</EuiButton>
)}
isOpen={this.state.isPopoverOpen3}
closePopover={this.closePopover3.bind(this)}
anchorPosition="upCenter"
withTitle
panelPaddingSize="none"
>
<EuiPopoverTitle>As the title, I keep my padding</EuiPopoverTitle>
<div style={{ width: '300px' }}>
<EuiText>
<p>
Popover content
</p>
</EuiText>
</div>
</EuiPopover>
</EuiFlexItem>
</EuiFlexGroup>
);
}
}
25 changes: 15 additions & 10 deletions src/components/panel/_panel.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
/**
* Padding map referenced in:
* - Popover
*/
$euiPanelPaddingModifiers: (
"--paddingSmall": $euiSizeS,
"--paddingMedium": $euiSize,
"--paddingLarge": $euiSizeL
) !default;


.euiPanel {
@include euiBottomShadowSmall;

Expand All @@ -6,16 +17,10 @@
border-radius: $euiBorderRadius;
flex-grow: 1;

&.euiPanel--paddingSmall {
padding: $euiSizeS;
}

&.euiPanel--paddingMedium {
padding: $euiSize;
}

&.euiPanel--paddingLarge {
padding: $euiSizeL;
@each $modifier, $amount in $euiPanelPaddingModifiers {
&.euiPanel#{$modifier} {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I see your point. Could we make one last change and put the -- here instead of in the map? This will be inline with how we're structuring this kind of pattern elsewhere.

padding: $amount;
}
}

&.euiPanel--shadow {
Expand Down
6 changes: 5 additions & 1 deletion src/components/popover/_mixins.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
@mixin euiPopoverTitle {
background-color: $euiColorLightestShade;
border-bottom: $euiBorderThin;
padding: $euiSizeM;

// Subtract 1px from the border radius since it's inside another container that also has the border radius
// -- makes for better rounded corners
border-top-left-radius: $euiBorderRadius - 1px;
border-top-right-radius: $euiBorderRadius - 1px;
}
16 changes: 15 additions & 1 deletion src/components/popover/_popover_title.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
@import '../panel/panel'; // grab the $euiPanelPaddingModifiers map
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move the variables into a panel/variables.scss file? Then it'd be clearer what we're importing, similar to the way we import a different component's mixins in https://github.com/elastic/eui/blob/master/src/components/context_menu/_context_menu_panel.scss#L1


.euiPopoverTitle {
@include euiPopoverTitle;
}

// If the popover's containing panel has padding applied,
// ensure the title expands to cover that padding and
// take on the same amount of padding horizontally

@each $modifier, $amount in $euiPanelPaddingModifiers {
.euiPopover__panel.euiPanel#{$modifier} & {
padding: $euiSizeM $amount;
margin: ($amount * -1) ($amount * -1) $amount;
}
}

}