Skip to content

Commit

Permalink
[EuiPopover] Resolve Positioning Errors When EuiPopover is Nested Ins…
Browse files Browse the repository at this point in the history
…ide of EuiFlyout (#5155)

* Refactored scroll method inside of EuiPopover to resolve positioning errors when components the popover are nested inside of containers like EuiFlyout

* Updated CHANGELOG with details from PR

* Added the repositionOnScroll prop to the an additional EuiPopover within the Flyout examples. Added additional documention about the repositionOnScroll prop

* Removed a rogue console.log statement

* Added repositionOnScroll prop to additional EuiInputPopover in the Complicated Flyout example in docs

Co-authored-by: Brianna Hall <briannahall@Briannas-MacBook-Pro.local>
  • Loading branch information
breehall and Brianna Hall committed Sep 13, 2021
1 parent 716ecc9 commit 6d38358
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ No public interface changes since `37.7.0`.
- Refactored styles of `EuiTabs` ([#5135](https://github.com/elastic/eui/pull/5135))
- Removed Sass variables for `EuiTabs` font size (`$euiTabFontSize, $euiTabFontSizeS, $euiTabFontSizeL`) ([#5135](https://github.com/elastic/eui/pull/5135))
- Extended all `EuiTabProps` for each `EuiTabbedContentTab` ([#5135](https://github.com/elastic/eui/pull/5135))
- Changed `EuiPopover`'s `repositionOnScroll` function to prevent popover and input elements from separating on scroll when nested in `EuiFlyout` ([#5155](https://github.com/elastic/eui/pull/5155))
- Added the `repositionOnScroll` prop to `EuiSuperSelect` ([#5155](https://github.com/elastic/eui/pull/5155))
- Added `useGeneratedHtmlId` utility, which memoizes the randomly generated ID on mount and prevents regenerated IDs on component rerender ([#5133](https://github.com/elastic/eui/pull/5133))
- Fixed `z-index` styles that were causing parts of `EuiResizableContainer` to overlap `EuiHeader` ([#5164](https://github.com/elastic/eui/pull/5164))

Expand Down
8 changes: 8 additions & 0 deletions src-docs/src/views/flyout/flyout_complicated.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React, { useState, Fragment } from 'react';
import {
EuiButton,
EuiButtonEmpty,
EuiCode,
EuiCodeBlock,
EuiComboBox,
EuiExpression,
Expand Down Expand Up @@ -207,11 +208,16 @@ export default () => {
</EuiButton>
}
isOpen={isPopoverOpen}
repositionOnScroll={true}
>
<p>
This is the popover content, notice how it can overflow the
flyout!
</p>
<p>
When placed in a flyout, the <EuiCode>repositionOnScroll</EuiCode>{' '}
prop ensures that the popover scrolls with body of the flyout.
</p>
</EuiPopover>
<EuiSpacer size="m" />
<EuiForm component="form">
Expand All @@ -221,6 +227,7 @@ export default () => {
valueOfSelected={superSelectvalue}
onChange={(value) => onSuperSelectChange(value)}
itemLayoutAlign="top"
repositionOnScroll={true}
hasDividers
/>
</EuiFormRow>
Expand All @@ -229,6 +236,7 @@ export default () => {
<EuiPopover
isOpen={isExpressionOpen}
closePopover={() => setIsExpressionOpen(false)}
repositionOnScroll={true}
button={
<EuiExpression
description="expression"
Expand Down
10 changes: 10 additions & 0 deletions src/components/form/super_select/super_select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@ export type EuiSuperSelectProps<T extends string> = CommonProps &
* Controls whether the options are shown. Default: false
*/
isOpen?: boolean;

/**
* When `true`, the popover's position is re-calculated when the user
* scrolls, this supports having fixed-position popover anchors. This value is passed
* to the EuiInputPopover component. When nesting an `EuiSuperSelect` in a scrollable container,
* `repositionOnScroll` should be `true`
*/
repositionOnScroll?: boolean;
};

export class EuiSuperSelect<T extends string> extends Component<
Expand Down Expand Up @@ -252,6 +260,7 @@ export class EuiSuperSelect<T extends string> extends Component<
fullWidth,
popoverClassName,
compressed,
repositionOnScroll,
...rest
} = this.props;

Expand Down Expand Up @@ -318,6 +327,7 @@ export class EuiSuperSelect<T extends string> extends Component<
closePopover={this.closePopover}
panelPaddingSize="none"
fullWidth={fullWidth}
repositionOnScroll={repositionOnScroll}
>
<EuiScreenReaderOnly>
<p role="alert">
Expand Down
9 changes: 5 additions & 4 deletions src/components/popover/popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ export interface EuiPopoverProps {
popoverRef?: Ref<HTMLDivElement>;
/**
* When `true`, the popover's position is re-calculated when the user
* scrolls, this supports having fixed-position popover anchors
* scrolls, this supports having fixed-position popover anchors. When nesting
* an `EuiPopover` in a scrollable container, `repositionOnScroll` should be `true`
*/
repositionOnScroll?: boolean;
/**
Expand Down Expand Up @@ -510,7 +511,7 @@ export class EuiPopover extends Component<Props, State> {
}

if (this.props.repositionOnScroll) {
window.addEventListener('scroll', this.positionPopoverFixed);
window.addEventListener('scroll', this.positionPopoverFixed, true);
}
}

Expand All @@ -523,9 +524,9 @@ export class EuiPopover extends Component<Props, State> {
// update scroll listener
if (prevProps.repositionOnScroll !== this.props.repositionOnScroll) {
if (this.props.repositionOnScroll) {
window.addEventListener('scroll', this.positionPopoverFixed);
window.addEventListener('scroll', this.positionPopoverFixed, true);
} else {
window.removeEventListener('scroll', this.positionPopoverFixed);
window.removeEventListener('scroll', this.positionPopoverFixed, true);
}
}

Expand Down

0 comments on commit 6d38358

Please sign in to comment.