fix(Popover): updated appendTo to inline by default (#8621)

* fix(Popover): updated appendTo to inline by default * Updated unit and integration tests * Attempted fix for build failure * Updated prop description * Updated appendTo based on new Popper logic
patternfly · Feb 10, 2023 · f6522ca · f6522ca
1 parent d2f3fbc
commit f6522ca
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 19 deletions.
diff --git a/packages/react-core/src/components/Popover/Popover.tsx b/packages/react-core/src/components/Popover/Popover.tsx
@@ -48,8 +48,8 @@ export interface PopoverProps {
   alertSeverityVariant?: 'default' | 'info' | 'warning' | 'success' | 'danger';
   /** The duration of the CSS fade transition animation. */
   animationDuration?: number;
-  /** The element to append the popover to. Defaults to "document.body". */
-  appendTo?: HTMLElement | ((ref?: HTMLElement) => HTMLElement);
+  /** The element to append the popover to. Defaults to "inline". */
+  appendTo?: HTMLElement | ((ref?: HTMLElement) => HTMLElement) | 'inline';
   /** Accessible label for the popover, required when header is not present. */
   'aria-label'?: string;
   /**
@@ -232,7 +232,7 @@ export const Popover: React.FunctionComponent<PopoverProps> = ({
   alertSeverityVariant,
   alertSeverityScreenReaderText,
   footerContent = null,
-  appendTo = () => document.body,
+  appendTo = 'inline',
   hideOnOutsideClick = true,
   onHide = (): void => null,
   onHidden = (): void => null,

diff --git a/packages/react-integration/cypress/integration/popover.spec.ts b/packages/react-integration/cypress/integration/popover.spec.ts
@@ -21,7 +21,7 @@ describe('Popover Demo Test', () => {
   });
 
   it('Popover header has correct default size', () => {
-    cy.get('div[id="popoverTarget"]').then((popoverLink: JQuery<HTMLDivElement>) => {
+    cy.get('button[id="popoverTarget"]').then((popoverLink: JQuery<HTMLDivElement>) => {
       cy.wrap(popoverLink).click();
       cy.get('.pf-c-popover').should('exist');
       cy.get('h6').should('have.class', 'pf-m-md');

diff --git a/packages/react-integration/demo-app-ts/src/components/demos/FormDemo/FormDemo.tsx b/packages/react-integration/demo-app-ts/src/components/demos/FormDemo/FormDemo.tsx
@@ -114,7 +114,12 @@ export class FormDemo extends Component<FormProps, FormState> {
             labelIcon={
               <Popover
                 headerContent={<div>The age of a person</div>}
-                bodyContent={<div>Age is typically measured in years.</div>}
+                bodyContent={
+                  <div>
+                    Age is typically measured in years. It is also common to measure age in months for newborns, e.g. 18
+                    months.
+                  </div>
+                }
               >
                 <button
                   id="helper-text-target"

diff --git a/packages/react-integration/demo-app-ts/src/components/demos/PopoverDemo/PopoverDemo.tsx b/packages/react-integration/demo-app-ts/src/components/demos/PopoverDemo/PopoverDemo.tsx
@@ -6,8 +6,7 @@ export class PopoverDemo extends Component {
   myPopoverProps = {
     headerContent: <div>Popover Header</div>,
     bodyContent: <div>Popover Body</div>,
-    footerContent: 'Popover Footer',
-    children: <div id="popoverTarget">Hello</div>
+    footerContent: 'Popover Footer'
   };
 
   constructor(props: any) {
@@ -20,21 +19,18 @@ export class PopoverDemo extends Component {
   }
 
   render() {
+    const { headerContent, bodyContent, footerContent } = this.myPopoverProps;
     return (
       <>
-        <Popover
-          headerContent={this.myPopoverProps.headerContent}
-          bodyContent={this.myPopoverProps.bodyContent}
-          footerContent={this.myPopoverProps.footerContent}
-        >
-          {this.myPopoverProps.children}
+        <Popover headerContent={headerContent} bodyContent={bodyContent} footerContent={footerContent}>
+          <button id="popoverTarget">Popover attached to children</button>
         </Popover>
         <div>
           <button id="popover-selector">Popover attached via selector ref</button>
           <Popover
-            headerContent={this.myPopoverProps.headerContent}
-            bodyContent={this.myPopoverProps.bodyContent}
-            footerContent={this.myPopoverProps.footerContent}
+            headerContent={headerContent}
+            bodyContent={bodyContent}
+            footerContent={footerContent}
             reference={() => document.getElementById('popover-selector')}
           />
         </div>
@@ -43,16 +39,18 @@ export class PopoverDemo extends Component {
             Popover attached via react ref
           </button>
           <Popover
-            headerContent={this.myPopoverProps.headerContent}
-            bodyContent={this.myPopoverProps.bodyContent}
-            footerContent={this.myPopoverProps.footerContent}
+            position="right-start"
+            headerContent={headerContent}
+            bodyContent={bodyContent}
+            footerContent={footerContent}
             reference={this.popoverRef}
           />
         </div>
         <Popover
           id="popover-content-close"
           aria-label="Popover with button in the body that can close it"
           headerContent={<div>Popover header</div>}
+          position="right-start"
           bodyContent={hide => (
             <div>
               <div>
@@ -88,10 +86,13 @@ export class PopoverDemo extends Component {
           bodyContent="I have a custom heading level."
           footerContent="Popover footer"
           headerComponent="h1"
+          position="auto"
         >
           <button id="popover-heading-level-toggle">Toggle Popover</button>
         </Popover>
       </>
     );
   }
 }
+
+export default PopoverDemo;