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.

Sign up for GitHub

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

Jump to bottom

fix(react): IonNav apply properties to page components #25603

Merged
merged 17 commits into from
Jul 28, 2022
Merged

fix(react): IonNav apply properties to page components #25603

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
e637993
fix(react): IonNav rootParams apply to root component
sean-perkins Jul 11, 2022
e47487e
test(react): IonNav rootParams
sean-perkins Jul 11, 2022
7b3751f
fix(): dev warning if using rootParams
sean-perkins Jul 11, 2022
882cf2a
test(): update test description
sean-perkins Jul 11, 2022
5d82c05
fix(react): improve IonNav with hmr
sean-perkins Jul 12, 2022
926fa9e
chore(): lint fixes
sean-perkins Jul 12, 2022
d0ed1de
Merge branch 'main' into FW-1839
sean-perkins Jul 12, 2022
9757b37
fix(): rootParams and componentProps work with IonNav
sean-perkins Jul 15, 2022
7be2d80
Merge branch 'main' into FW-1839
sean-perkins Jul 15, 2022
041de9e
fix(react): can assign ref to IonNav (#25653)
sean-perkins Jul 20, 2022
116491c
Merge branch 'main' into FW-1839
sean-perkins Jul 20, 2022
93a1ed7
Merge branch 'main' into FW-1839
sean-perkins Jul 25, 2022
e8eabf1
Merge branch 'main' into FW-1839
sean-perkins Jul 26, 2022
9d97bff
Merge branch 'main' into FW-1839
liamdebeasi Jul 27, 2022
85220d2
Merge branch 'main' into FW-1839
sean-perkins Jul 28, 2022
d9bfdfa
Merge branch 'main' into FW-1839
sean-perkins Jul 28, 2022
787fdfe
Merge branch 'main' into FW-1839
sean-perkins Jul 28, 2022
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
6 changes: 3 additions & 3 deletions packages/react/src/components/navigation/IonNav.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ const IonNavInner = createReactComponent<
>('ion-nav', undefined, undefined, defineCustomElement);

export const IonNav: React.FC<JSX.IonNav> = ({ children, ...restOfProps }) => {
const [views, setViews] = useState<React.ReactPortal[]>([]);
const [views, setViews] = useState<React.ReactElement[]>([]);

/**
* Allows us to create React components that are rendered within
* the context of the IonNav component.
*/
const addView = (view: React.ReactPortal) => setViews([...views, view]);
const removeView = (view: React.ReactPortal) => setViews(views.filter((v) => v !== view));
const addView = (view: React.ReactElement) => setViews([...views, view]);
const removeView = (view: React.ReactElement) => setViews(views.filter((v) => v !== view));

const delegate = ReactDelegate(addView, removeView);

Expand Down
23 changes: 14 additions & 9 deletions packages/react/src/framework-delegate.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,38 @@
import { FrameworkDelegate } from '@ionic/core/components';
import { createPortal } from 'react-dom';

type ReactComponent = (props?: any) => JSX.Element;

export const ReactDelegate = (
addView: (view: React.ReactPortal) => void,
removeView: (view: React.ReactPortal) => void
addView: (view: React.ReactElement) => void,
removeView: (view: React.ReactElement) => void
): FrameworkDelegate => {
let Component: React.ReactPortal;
const refMap = new WeakMap<ReactComponent, React.ReactElement>();

const attachViewToDom = async (
parentElement: HTMLElement,
component: () => JSX.Element,
component: ReactComponent,
propsOrDataObj?: any,
cssClasses?: string[]
): Promise<any> => {
const div = document.createElement('div');
cssClasses && div.classList.add(...cssClasses);
parentElement.appendChild(div);

Component = createPortal(component(), div);
const componentWithProps = component(propsOrDataObj);
const hostComponent = createPortal(componentWithProps, div);

Component.props = propsOrDataObj;
refMap.set(component, hostComponent);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need the map?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One instance of the delegate is constructed for one instance of IonNav, but IonNav can attach multiple views to the DOM.

The map allows us to keep track of all instances of the individual views that were pushed inside the Nav. We can then do a look-up against the map to find the correct instance to remove the view from the DOM.

In the previous implementation, the value of Component would be reset when a new view was pushed, which prevented the previous view from ever being removed.

Alternatively you could use an object instead of a map here, but since we are associating to a React component, a WeakMap would be my recommendation here so that it does not interfere with garbage collection.


addView(Component);
addView(hostComponent);

return Promise.resolve(div);
};

const removeViewFromDom = (): Promise<void> => {
Component && removeView(Component);
const removeViewFromDom = (_container: any, component: ReactComponent): Promise<void> => {
const hostComponent = refMap.get(component);
hostComponent && removeView(hostComponent);

return Promise.resolve();
};

Expand Down
12 changes: 12 additions & 0 deletions packages/react/test-app/cypress/integration/navigation/IonNav.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,16 @@ describe('IonNav', () => {
cy.get('ion-nav').contains('Page one content');
});

it('should pass params to the page', () => {
cy.get('#pageOneProps').should('have.text', '{"someString":"Hello","someNumber":3,"someBoolean":true}');
});

it('should pass componentProps to sub pages', () => {
cy.get('ion-button').contains('Go to Page Two').click();

cy.get('#pageTwoContent').should('be.visible');

cy.get('#pageTwoProps').should('have.text', '{"someValue":"Hello"}');
});

});
131 changes: 72 additions & 59 deletions packages/react/test-app/src/pages/navigation/NavComponent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,70 +13,83 @@ import {
} from '@ionic/react';
import React from 'react';

const PageOne = (props: { someString: string; someNumber: number; someBoolean: boolean }) => {
return (
<>
<IonHeader>
<IonToolbar>
<IonTitle>Page One</IonTitle>
<IonButtons>
<IonBackButton />
</IonButtons>
</IonToolbar>
</IonHeader>
<IonContent id="pageOneContent">
<IonLabel>Page one content</IonLabel>
<div id="pageOneProps">{JSON.stringify(props)}</div>
<IonNavLink
routerDirection="forward"
component={PageTwo}
componentProps={{ someValue: 'Hello' }}
>
<IonButton>Go to Page Two</IonButton>
</IonNavLink>
</IonContent>
</>
);
};

const PageTwo = (props?: { someValue: string }) => {
return (
<>
<IonHeader>
<IonToolbar>
<IonTitle>Page Two</IonTitle>
<IonButtons>
<IonBackButton />
</IonButtons>
</IonToolbar>
</IonHeader>
<IonContent id="pageTwoContent">
<IonLabel>Page two content</IonLabel>
<div id="pageTwoProps">{JSON.stringify(props)}</div>
<IonNavLink routerDirection="forward" component={PageThree}>
<IonButton>Go to Page Three</IonButton>
</IonNavLink>
</IonContent>
</>
);
};

const PageThree = () => {
return (
<>
<IonHeader>
<IonToolbar>
<IonTitle>Page Three</IonTitle>
<IonButtons>
<IonBackButton />
</IonButtons>
</IonToolbar>
</IonHeader>
<IonContent>
<IonLabel>Page three content</IonLabel>
</IonContent>
</>
);
};

const NavComponent: React.FC = () => {
return (
<IonPage>
<IonNav
root={() => {
return (
<>
<IonHeader>
<IonToolbar>
<IonTitle>Page One</IonTitle>
<IonButtons>
<IonBackButton />
</IonButtons>
</IonToolbar>
</IonHeader>
<IonContent id="pageOneContent">
<IonLabel>Page one content</IonLabel>
<IonNavLink
routerDirection="forward"
component={() => {
return (
<>
<IonHeader>
<IonToolbar>
<IonTitle>Page Two</IonTitle>
<IonButtons>
<IonBackButton />
</IonButtons>
</IonToolbar>
</IonHeader>
<IonContent id="pageTwoContent">
<IonLabel>Page two content</IonLabel>
<IonNavLink
routerDirection="forward"
component={() => (
<>
<IonHeader>
<IonToolbar>
<IonTitle>Page Three</IonTitle>
<IonButtons>
<IonBackButton />
</IonButtons>
</IonToolbar>
</IonHeader>
<IonContent>
<IonLabel>Page three content</IonLabel>
</IonContent>
</>
)}
>
<IonButton>Go to Page Three</IonButton>
</IonNavLink>
</IonContent>
</>
);
}}
>
<IonButton>Go to Page Two</IonButton>
</IonNavLink>
</IonContent>
</>
);
root={PageOne}
rootParams={{
someString: 'Hello',
someNumber: 3,
someBoolean: true,
}}
></IonNav>
/>
</IonPage>
);
};
Expand Down