From a176136f669a1ede0c78f5cfaea8f55371b92397 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Mon, 30 Jul 2018 15:38:05 -0600 Subject: [PATCH 1/6] Fix bug where a position: fixed popover content doesn't scroll --- src-docs/src/views/popover/popover_example.js | 20 +++++++ src-docs/src/views/popover/popover_fixed.js | 58 +++++++++++++++++++ src/components/popover/popover.js | 3 + 3 files changed, 81 insertions(+) create mode 100644 src-docs/src/views/popover/popover_fixed.js diff --git a/src-docs/src/views/popover/popover_example.js b/src-docs/src/views/popover/popover_example.js index 4b86b9d15ed..c0c81e9c907 100644 --- a/src-docs/src/views/popover/popover_example.js +++ b/src-docs/src/views/popover/popover_example.js @@ -44,6 +44,9 @@ import PopoverContainer from './popover_container'; const popoverContainerSource = require('!!raw-loader!./popover_container'); const popoverContainerHtml = renderToHtml(PopoverContainer); +import PopoverFixed from './popover_fixed'; +const popoverFixedSource = require('!!raw-loader!./popover_fixed'); +const popoverFixedHtml = renderToHtml(PopoverFixed); export const PopoverExample = { title: 'Popover', @@ -204,5 +207,22 @@ export const PopoverExample = { ), demo: , + }, { + title: 'Popover on a static element', + source: [{ + type: GuideSectionTypes.JS, + code: popoverFixedSource, + }, { + type: GuideSectionTypes.HTML, + code: popoverFixedHtml, + }], + text: ( +
+

+ Popover content even works on position: static; elements. +

+
+ ), + demo: , }], }; diff --git a/src-docs/src/views/popover/popover_fixed.js b/src-docs/src/views/popover/popover_fixed.js new file mode 100644 index 00000000000..16a418f7a99 --- /dev/null +++ b/src-docs/src/views/popover/popover_fixed.js @@ -0,0 +1,58 @@ +import React, { + Component, +} from 'react'; + +import { + EuiButton, + EuiPopover, +} from '../../../../src/components'; + +export default class PopoverContainer extends Component { + constructor(props) { + super(props); + + this.state = { + isPopoverOpen: false, + }; + } + + onButtonClick = () => { + this.setState({ + isPopoverOpen: !this.state.isPopoverOpen, + }); + } + + closePopover = () => { + this.setState({ + isPopoverOpen: false, + }); + } + + setPanelRef = node => this.panel = node; + + render() { + const button = ( + + Show fixed popover + + ); + + return ( + +
+ This popover scrolls with the button element! +
+ + ); + } +} diff --git a/src/components/popover/popover.js b/src/components/popover/popover.js index 3dd0fd9e970..0bfffff0fc5 100644 --- a/src/components/popover/popover.js +++ b/src/components/popover/popover.js @@ -153,6 +153,8 @@ export class EuiPopover extends Component { this.setState({ suppressingPopover: false, isOpening: true }); // eslint-disable-line react/no-did-mount-set-state } + window.addEventListener('scroll', this.positionPopover); + this.updateFocus(); } @@ -184,6 +186,7 @@ export class EuiPopover extends Component { } componentWillUnmount() { + window.removeEventListener('scroll', this.positionPopover); clearTimeout(this.closingTransitionTimeout); } From e51dfc6ef0277baec146c9de5ca568dd933cc964 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Mon, 30 Jul 2018 15:43:03 -0600 Subject: [PATCH 2/6] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 783fee92995..d0ad14f7bec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ **Bug fixes** - Fixed `EuiXYChart` responsive resize in a flexbox layout ([#1041](https://github.com/elastic/eui/pull/1041)) +- Updated `EuiPopover` to reposition content when the window is scrolled. ([#1062](https://github.com/elastic/eui/pull/1062)) ## [`3.2.1`](https://github.com/elastic/eui/tree/v3.2.1) From f01eefdfde1bc52b6fc8a0e1c849fe554e7148f1 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Wed, 1 Aug 2018 10:51:52 -0600 Subject: [PATCH 3/6] Add a repositionOnScroll prop to EuiPopover --- CHANGELOG.md | 3 ++- src-docs/src/views/popover/popover_fixed.js | 1 + src/components/popover/popover.js | 14 +++++++++++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0ad14f7bec..b908103573d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,10 @@ ## [`master`](https://github.com/elastic/eui/tree/master) +- Add `repositionOnScroll` prop to `EuiPopover` which enables repositioning the popover when the window is scrolled. ([#1062](https://github.com/elastic/eui/pull/1062)) + **Bug fixes** - Fixed `EuiXYChart` responsive resize in a flexbox layout ([#1041](https://github.com/elastic/eui/pull/1041)) -- Updated `EuiPopover` to reposition content when the window is scrolled. ([#1062](https://github.com/elastic/eui/pull/1062)) ## [`3.2.1`](https://github.com/elastic/eui/tree/v3.2.1) diff --git a/src-docs/src/views/popover/popover_fixed.js b/src-docs/src/views/popover/popover_fixed.js index 16a418f7a99..ad485ef23b1 100644 --- a/src-docs/src/views/popover/popover_fixed.js +++ b/src-docs/src/views/popover/popover_fixed.js @@ -48,6 +48,7 @@ export default class PopoverContainer extends Component { isOpen={this.state.isPopoverOpen} closePopover={this.closePopover} style={{ position: 'fixed', bottom: 50, right: 50 }} + repositionOnScroll={true} >
This popover scrolls with the button element! diff --git a/src/components/popover/popover.js b/src/components/popover/popover.js index 0bfffff0fc5..3b3069dfe27 100644 --- a/src/components/popover/popover.js +++ b/src/components/popover/popover.js @@ -153,7 +153,9 @@ export class EuiPopover extends Component { this.setState({ suppressingPopover: false, isOpening: true }); // eslint-disable-line react/no-did-mount-set-state } - window.addEventListener('scroll', this.positionPopover); + if (this.props.repositionOnScroll) { + window.addEventListener('scroll', this.positionPopover); + } this.updateFocus(); } @@ -171,6 +173,15 @@ export class EuiPopover extends Component { }); } + // update scroll listener + if (prevProps.repositionOnScroll !== this.props.repositionOnScroll) { + if (this.props.repositionOnScroll) { + window.addEventListener('scroll', this.positionPopover); + } else { + window.removeEventListener('scroll', this.positionPopover); + } + } + // The popover is being closed. if (prevProps.isOpen && !this.props.isOpen) { // If the user has just closed the popover, queue up the removal of the content after the @@ -424,6 +435,7 @@ EuiPopover.propTypes = { PropTypes.node, PropTypes.instanceOf(HTMLElement) ]), + repositionOnScroll: PropTypes.bool, }; EuiPopover.defaultProps = { From 4369a5f84399ea5d5f910dc51dd2840803b5fa28 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Wed, 1 Aug 2018 10:54:43 -0600 Subject: [PATCH 4/6] correct the changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78ef1b06c5b..3aa00a1cf51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ - `EuiComboBox` now applies the provided `data-test-subj` to its options list element with the suffix `-optionsList` so you can find a specific combo box instance's options list. This wasn't previously possible because the options list is attached to the body element, not the combo box element. This is in addition to the existing `data-test-subj="comboBoxOptionsList"`. ([#1054](https://github.com/elastic/eui/pull/1054)) - EUI now provides minified versions of the themes' CSS files. ([#1070](https://github.com/elastic/eui/pull/1070)) -- Add `repositionOnScroll` prop to `EuiPopover` which enables repositioning the popover when the window is scrolled. ([#1062](https://github.com/elastic/eui/pull/1062)) +- Add `repositionOnScroll` prop to `EuiPopover` which enables repositioning the popover when the window is scrolled. ([#1064](https://github.com/elastic/eui/pull/1064)) **Bug fixes** From e61097ff8a5206a67ec3f04e4d047125dcb8ce05 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 3 Aug 2018 11:30:09 -0600 Subject: [PATCH 5/6] addressing PR feedback --- src-docs/src/views/popover/popover_example.js | 4 +-- src-docs/src/views/popover/popover_fixed.js | 30 ++++++++++++------- src/components/popover/popover.js | 1 + 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src-docs/src/views/popover/popover_example.js b/src-docs/src/views/popover/popover_example.js index c0c81e9c907..ac41ed3dedb 100644 --- a/src-docs/src/views/popover/popover_example.js +++ b/src-docs/src/views/popover/popover_example.js @@ -208,7 +208,7 @@ export const PopoverExample = { ), demo: , }, { - title: 'Popover on a static element', + title: 'Popover on a fixed element', source: [{ type: GuideSectionTypes.JS, code: popoverFixedSource, @@ -219,7 +219,7 @@ export const PopoverExample = { text: (

- Popover content even works on position: static; elements. + Popover content even works on position: fixed; elements.

), diff --git a/src-docs/src/views/popover/popover_fixed.js b/src-docs/src/views/popover/popover_fixed.js index ad485ef23b1..f543a513353 100644 --- a/src-docs/src/views/popover/popover_fixed.js +++ b/src-docs/src/views/popover/popover_fixed.js @@ -12,10 +12,13 @@ export default class PopoverContainer extends Component { super(props); this.state = { + isExampleShown: false, isPopoverOpen: false, }; } + toggleExample = () => this.setState(({ isExampleShown }) => ({ isExampleShown: !isExampleShown })) + onButtonClick = () => { this.setState({ isPopoverOpen: !this.state.isPopoverOpen, @@ -43,17 +46,22 @@ export default class PopoverContainer extends Component { ); return ( - -
- This popover scrolls with the button element! -
- + + Toggle Example + {this.state.isExampleShown && ( + +
+ This popover scrolls with the button element! +
+ + )} + ); } } diff --git a/src/components/popover/popover.js b/src/components/popover/popover.js index 27f5ace44b3..8a0db98038c 100644 --- a/src/components/popover/popover.js +++ b/src/components/popover/popover.js @@ -434,6 +434,7 @@ EuiPopover.propTypes = { PropTypes.node, PropTypes.instanceOf(HTMLElement) ]), + /** When `true`, the popover's position is re-calculated when the user scrolls, this supports having fixed-position popover anchors. */ repositionOnScroll: PropTypes.bool, }; From 74999dcae3a4af4aa1c90c522b302dbedb7712f7 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 3 Aug 2018 11:41:27 -0600 Subject: [PATCH 6/6] update changelog diff --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a70d0b40308..de32fefe811 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ - Added `spacesApp` logo to `EuiIcon` set ([#1065](https://github.com/elastic/eui/pull/1065)) - Added `!default` to border SASS props ([#1079](https://github.com/elastic/eui/pull/1079)) +- Added `repositionOnScroll` prop to `EuiPopover` which enables repositioning the popover when the window is scrolled. ([#1064](https://github.com/elastic/eui/pull/1064)) **Bug fixes** @@ -15,7 +16,6 @@ - Added `onTableChange` callback to `EuiInMemoryTable` which notifies on sorting and pagination changes. ([#1060](https://github.com/elastic/eui/pull/1060)) - `EuiComboBox` now applies the provided `data-test-subj` to its options list element with the suffix `-optionsList` so you can find a specific combo box instance's options list. This wasn't previously possible because the options list is attached to the body element, not the combo box element. This is in addition to the existing `data-test-subj="comboBoxOptionsList"`. ([#1054](https://github.com/elastic/eui/pull/1054)) - EUI now provides minified versions of the themes' CSS files. ([#1070](https://github.com/elastic/eui/pull/1070)) -- Add `repositionOnScroll` prop to `EuiPopover` which enables repositioning the popover when the window is scrolled. ([#1064](https://github.com/elastic/eui/pull/1064)) **Bug fixes**