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(react): inline overlays can be conditionally rendered #26111

Merged
merged 14 commits into from
Oct 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 36 additions & 22 deletions packages/react/src/components/createInlineOverlayComponent.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { OverlayEventDetail } from '@ionic/core/components'
import { OverlayEventDetail } from '@ionic/core/components';
import React, { createElement } from 'react';

import {
Expand All @@ -12,7 +12,7 @@ import { createForwardRef } from './utils';

type InlineOverlayState = {
isOpen: boolean;
}
};

interface IonicReactInternalProps<ElementType> extends React.HTMLAttributes<ElementType> {
forwardedRef?: React.ForwardedRef<ElementType>;
Expand All @@ -32,17 +32,20 @@ export const createInlineOverlayComponent = <PropType, ElementType>(
defineCustomElement();
}
const displayName = dashToPascalCase(tagName);
const ReactComponent = class extends React.Component<IonicReactInternalProps<PropType>, InlineOverlayState> {
const ReactComponent = class extends React.Component<
IonicReactInternalProps<PropType>,
InlineOverlayState
> {
ref: React.RefObject<HTMLElement>;
wrapperRef: React.RefObject<HTMLElement>;
stableMergedRefs: React.RefCallback<HTMLElement>
stableMergedRefs: React.RefCallback<HTMLElement>;

constructor(props: IonicReactInternalProps<PropType>) {
super(props);
// Create a local ref to to attach props to the wrapped element.
this.ref = React.createRef();
// React refs must be stable (not created inline).
this.stableMergedRefs = mergeRefs(this.ref, this.props.forwardedRef)
this.stableMergedRefs = mergeRefs(this.ref, this.props.forwardedRef);
// Component is hidden by default
this.state = { isOpen: false };
// Create a local ref to the inner child element.
Expand Down Expand Up @@ -123,23 +126,34 @@ export const createInlineOverlayComponent = <PropType, ElementType>(
style,
};

/**
* We only want the inner component
* to be mounted if the overlay is open,
* so conditionally render the component
* based on the isOpen state.
*/
return createElement(tagName, newProps, (this.state.isOpen || this.props.keepContentsMounted) ?
createElement('div', {
id: 'ion-react-wrapper',
ref: this.wrapperRef,
style: {
display: 'flex',
flexDirection: 'column',
height: '100%'
}
}, children) :
null
return createElement(
'template',
{},
createElement(
tagName,
newProps,
/**
* We only want the inner component
* to be mounted if the overlay is open,
* so conditionally render the component
* based on the isOpen state.
*/
this.state.isOpen || this.props.keepContentsMounted
? createElement(
'div',
{
id: 'ion-react-wrapper',
ref: this.wrapperRef,
style: {
display: 'flex',
flexDirection: 'column',
height: '100%',
},
},
children
)
: null
)
);
}

Expand Down
14 changes: 14 additions & 0 deletions packages/react/test-app/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ import OverlayComponents from './pages/overlay-components/OverlayComponents';
import KeepContentsMounted from './pages/overlay-components/KeepContentsMounted';
import Tabs from './pages/Tabs';
import NavComponent from './pages/navigation/NavComponent';
import IonModalConditionalSibling from './pages/overlay-components/IonModalConditionalSibling';
import IonModalConditional from './pages/overlay-components/IonModalConditional';
import IonModalDatetimeButton from './pages/overlay-components/IonModalDatetimeButton';
import IonPopoverNested from './pages/overlay-components/IonPopoverNested';

setupIonicReact();

Expand All @@ -37,6 +41,16 @@ const App: React.FC = () => (
<Route path="/" component={Main} />
<Route path="/overlay-hooks" component={OverlayHooks} />
<Route path="/overlay-components" component={OverlayComponents} />
<Route path="/overlay-components/nested-popover" component={IonPopoverNested} />
<Route
path="/overlay-components/modal-conditional-sibling"
component={IonModalConditionalSibling}
/>
<Route path="/overlay-components/modal-conditional" component={IonModalConditional} />
<Route
path="/overlay-components/modal-datetime-button"
component={IonModalDatetimeButton}
/>
<Route path="/keep-contents-mounted" component={KeepContentsMounted} />
<Route path="/navigation" component={NavComponent} />
<Route path="/tabs" component={Tabs} />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { IonButton, IonContent, IonModal } from '@ionic/react';
import { useRef } from 'react';
import { useState } from 'react';

/**
* Issue: https://github.com/ionic-team/ionic-framework/issues/25590
*
* Exception is thrown when IonModal is conditionally rendered inline.
*/
const IonModalConditional = () => {
const [showIonModal, setShowIonModal] = useState(false);
const [isOpen, setIsOpen] = useState(true);

const modal = useRef<HTMLIonModalElement>(null);

return (
<IonContent>
<IonButton
id="renderModalBtn"
onClick={() => {
setShowIonModal(true);
setIsOpen(true);
}}
>
Render Modal
</IonButton>
{showIonModal && (
<IonModal
ref={modal}
isOpen={isOpen}
onDidDismiss={() => {
setIsOpen(false);
setShowIonModal(false);
}}
>
<IonContent>
Modal Content
<IonButton id="dismissModalBtn" onClick={() => modal.current!.dismiss()}>
Close
</IonButton>
</IonContent>
</IonModal>
)}
</IonContent>
);
};

export default IonModalConditional;
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { IonButton, IonCard, IonContent, IonModal } from '@ionic/react';
import { useRef } from 'react';
import { useState } from 'react';

/**
* Issue: https://github.com/ionic-team/ionic-framework/issues/25590
*
* Exception is thrown when adding/removing nodes that are siblings of IonModal,
* while the modal is being dismissed.
*/
const IonModalConditionalSibling = () => {
const [items, setItems] = useState<string[]>(['Item 1']);
const [isOpen, setIsOpen] = useState(true);

const modal = useRef<HTMLIonModalElement>(null);

return (
<IonContent>
{items && items.map((item) => <IonCard key={item}>Before {item}</IonCard>)}
<IonModal
ref={modal}
isOpen={isOpen}
onWillDismiss={() => {
setItems([...items, `Item ${items.length + 1}`]);
}}
onDidDismiss={() => setIsOpen(false)}
>
<IonContent>
Modal Content
<IonButton onClick={() => modal.current!.dismiss()}>Close</IonButton>
</IonContent>
</IonModal>
{items && items.map((item) => <IonCard key={item}>After {item}</IonCard>)}
</IonContent>
);
};

export default IonModalConditionalSibling;
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { IonButton, IonContent, IonDatetime, IonDatetimeButton, IonModal } from '@ionic/react';
import { useRef } from 'react';
import { useState } from 'react';

const IonModalDatetimeButton = () => {
const [showIonModal, setShowIonModal] = useState(false);
const [isOpen, setIsOpen] = useState(true);

const modal = useRef<HTMLIonModalElement>(null);

return (
<IonContent>
<IonButton
id="renderModalBtn"
onClick={() => {
setShowIonModal(true);
setIsOpen(true);
}}
>
Render Modal
</IonButton>
{showIonModal && (
<IonModal
ref={modal}
isOpen={isOpen}
onDidDismiss={() => {
setIsOpen(false);
setShowIonModal(false);
}}
>
<IonContent>
Modal Content
<IonDatetimeButton datetime="startDate" />
<IonModal id="datetimeModal" keepContentsMounted={true}>
<IonDatetime
id="startDate"
preferWheel
presentation="date"
name="startDate"
showDefaultButtons
color="primary"
/>
</IonModal>
</IonContent>
</IonModal>
)}
</IonContent>
);
};

export default IonModalDatetimeButton;
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import {
IonButton,
IonContent,
IonIcon,
IonItem,
IonList,
IonListHeader,
IonPage,
IonPopover,
IonHeader,
IonTitle,
IonToolbar,
} from '@ionic/react';
import { arrowForward } from 'ionicons/icons';
import { useRef } from 'react';

const IonPopoverNested = () => {
const menuPopover = useRef<HTMLIonPopoverElement>(null);
const submenuPopover = useRef<HTMLIonPopoverElement>(null);

return (
<IonPage>
<IonHeader>
<IonToolbar>
<IonTitle>Nested Popover</IonTitle>
</IonToolbar>
</IonHeader>
<IonContent className="ion-padding">
<IonButton id="open">Show Popover</IonButton>
<IonPopover ref={menuPopover} id="menu-popover" trigger="open">
<IonList>
<IonListHeader>Menu Items</IonListHeader>
<IonItem>Item 1</IonItem>
<IonItem>Item 2</IonItem>
<IonItem>Item 3</IonItem>
<IonItem button id="item-4">
More
<IonIcon icon={arrowForward} slot="end" />
</IonItem>
<IonItem button id="close-menu-popover" onClick={() => menuPopover.current!.dismiss()}>
Close
</IonItem>
</IonList>
<IonPopover ref={submenuPopover} id="submenu-popover" trigger="item-4" side="right">
<IonList>
<IonListHeader>Submenu Items</IonListHeader>
<IonItem>Item 1</IonItem>
<IonItem>Item 2</IonItem>
<IonItem>Item 3</IonItem>
<IonItem
id="close-submenu-popover"
button
onClick={() => submenuPopover.current!.dismiss()}
>
Close
</IonItem>
</IonList>
</IonPopover>
</IonPopover>
</IonContent>
</IonPage>
);
};

export default IonPopoverNested;
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const KeepContentsMounted: React.FC = () => {
<IonButton id="open-modal" onClick={() => setShowModal(true)}>Open Modal</IonButton>
<IonButton id="open-popover" onClick={() => setShowPopover(true)}>Open Popover</IonButton>

<IonModal keepContentsMounted={true} id="default-modal" isOpen={showModal} onDidDismiss={() => setShowPopover(false)}>
<IonModal keepContentsMounted={true} id="default-modal" isOpen={showModal} onDidDismiss={() => setShowModal(false)}>
<IonContent>
<IonButton onClick={() => setShowModal(false)}>Dismiss</IonButton>
Modal Content
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,45 @@ describe('IonModal', () => {
cy.get('ion-modal div').contains('overriden value')
});
});

describe('IonModal: conditional rendering', () => {

// Issue: https://github.com/ionic-team/ionic-framework/issues/25590
it('should push a new IonCard when dismissed', () => {
cy.visit('/overlay-components/modal-conditional-sibling');
// Renders a card before and after the modal
cy.get('ion-card').should('have.length', 2);

cy.get('ion-button').click();

cy.get('ion-card').should('have.length', 4);
});

// Issue: https://github.com/ionic-team/ionic-framework/issues/25590
it('should be conditionally rendered', () => {
cy.visit('/overlay-components/modal-conditional');

cy.get('ion-modal').should('not.exist');
cy.get('ion-button#renderModalBtn').click();

cy.get('ion-modal').should('be.visible');

cy.get('ion-button#dismissModalBtn').click();
cy.get('ion-button#renderModalBtn').should('be.visible');
});

it('should display an inline modal within a modal', () => {
cy.visit('/overlay-components/modal-datetime-button');

cy.get('ion-modal').should('not.exist');
cy.get('ion-button#renderModalBtn').click();

cy.get('ion-modal').should('be.visible');

cy.get('ion-datetime-button').click();

cy.get('ion-modal#datetimeModal').should('be.visible');
cy.get('ion-datetime').should('be.visible');
});

});