bug: can't control Modal with router params about ionic-framework

[legendar]

Another scenario is if we have some filtering controls on the page that also change the URL path (to allow the user to share the URL), every filter change will now (after version 7.5.8) create a new view and reset the entire page state. So, I think the solution introduced in 1705d06 is not optimal as it introduces a lot of breaking changes.

[amandaejohnston]

Thank you for the issue. The original behavior that was changed in 1705d06 was a bug in Ionic Framework. Re-using the same view instance when navigating from one route to another is not a desired behavior; you can read more about the problems that arise in the original issue here: #26524 It sounds like your original solutions were relying on this bug in Ionic, so they will need updating now that the bug is fixed.

I can reproduce the behavior you've described in your linked reproduction, but the issue is due to the code not properly reacting to changes in the route parameters or updating isDetailsModalOpen accordingly. I was able to get everything working with the following changes:

+ import { useEffect, useState } from 'react';

- const {itemId} = useParams<{itemId: string}>();
- const isDetailsModalOpen = Boolean(itemId);
+ const params = useParams<{itemId: string}>();
+ const [isDetailsModalOpen, setDetailsModalOpen] = useState(false);

const closeDetails = () => {
  push("/items/", "none", "pop");
+ setDetailsModalOpen(false);
};

+ useEffect(() => {
+   setDetailsModalOpen(Boolean(params.itemId));
+ }, [params]);

// in the modal render
- <IonTitle>Item {itemId} details</IonTitle>
+ <IonTitle>Item {params.itemId} details</IonTitle>

Can you give this a try and let me know if you have any questions?

[legendar]

Thank you for the reply. Your solution works well in this particular case👍

However, it's not a solution but rather a workaround. It relies on the fact that params is a new object for every component render, and React uses Object.is to compare dependencies in useEffect. So useEffect will always be triggered in this case. But if you try to change the code slightly to make it more prettier, it stops working☹️:

- const params = useParams<{itemId: string}>();
+ const {itemId} = useParams<{itemId: string}>();

- useEffect(() => {
-   setDetailsModalOpen(Boolean(params.itemId));
- }, [params]);
+ useEffect(() => {
+   setDetailsModalOpen(Boolean(params.itemId));
+ }, [itemId]);

The issue is much bigger than just modal showing. It affects the entire component's state and makes the React state useless. For example, if we have a checkbox in a list, there is no simple way to pass this state into the modal without additional setState, which is not the React way. See the example:

const [items, setItems] = useState<number[]>([]);

const toggleItem = (e) => {
    e.stopPropagation();

    const id = e.target.dataset.id;

    if(e.target.checked) {
      setItems(items => [...items, id])
    } else {
      setItems(items => items.filter(i => i !== id))
    }
  }

then render the list of items with checkboxes:

<IonLabel><input type="checkbox" data-id={1} onClick={toggleItem} /> Item 1</IonLabel>

and see what happens:

state.mp4

Sure, we can find a workaround for this case as well, but it's not a solution. The actual issue is that Ionic creates a new view instance every time the route is changed. So, after clicking on an item in the list, the modal doesn't react to the parameters because it's another modal in another view instance. You can verify this by changing the push call to just push("/items/") instead of push("/items/", "none", "pop") ("none" added to suppress animation).

Basically, if I specify a parameterized route, I expect the same view to be re-rendered with a new parameter and this is how react works.

<Route path="/items/past/" exact={true}>
  <ItemsList filter="past" /> {/* <-- separate view because in a separate Route component */}
</Route>
<Route path="/items/upcoming/" exact={true}>
  <ItemsList filter="upcoming" /> {/* <-- separate view because in a separate Route component */}
</Route>
<Route path="/items/:itemId?/">
  <ItemsList /> {/* <-- always the same view */}
</Route>

Yes, I understand that in some situations it's good to have a new view item for animations, etc., as shown in #26524 or in any cases where the new route corresponds to a new page. However, in other scenarios like the one I described above, where we have one big page with some internal states or filtering, it's not a good idea.
So, there should probably be a mechanism to switch between the two behaviors instead of pushing one that forces us to find a lot of workarounds for other scenarios.

[liamdebeasi]

The actual issue is that Ionic creates a new view instance every time the route is changed.

This is the intended behavior, though it differs slightly from how web applications work. Normally in web apps the JS Framework will reuse the component instance, as you noted. For example, if /page/:id maps to Foo, going from /page/1 to /page/2 will re-use the same instance of Foo. For web apps, this is fine. For mobile apps it causes issues due to how stack based navigation works.

As an example, say you are on /page/1 and you enter your email into a text input. You press a button that brings you to /page/2. Typically in a mobile app, the view for /page/2 should be separate from /page/1. Additionally, you should be able to swipe back from /page/2 to /page/1 and still see your email entered on the first view. If we reused the view instance that native mobile paradigm breaks. More specifically, you would not be able to swipe back to /page/1, and the email you entered into the text input would be missing.

---

I would advise against coupling modals to routing like this. Modals are temporary dialogs that are dependent on the context in which they are presented. By coupling the modal to the global state of your app (the URL), you limit how and when you can show this dialog. A lot of the complexity here could be avoided by pushing a new view with a route instead of trying to couple the modal to the route.

Part of the issue here is how views are being pushed in order to get around the modal/routing behavior. As I noted, I would recommend avoiding this entirely. But if you need to do it this way, here are some adjustments you can make to your current component. I added some documentation to explain some of the changes I made.

import { IonContent, IonHeader, IonPage, IonTitle, IonToolbar, IonList, IonItem, IonLabel, IonModal, useIonRouter, IonButtons, IonButton, createAnimation } from '@ionic/react';
import { useParams } from "react-router";

/**
  Setting routerDirection="none" seems to cause new views to be mounted
  and then never unmounted which is contributing to the problem here. To
  avoid this, we pass in a blank animation to skip the transition effect.
  Seems related to https://github.com/ionic-team/ionic-framework/issues/24074
*/
const animation = () => createAnimation();

export default function ItemsList() {
  const { itemId } = useParams<{itemId: string}>();

  /**
   * Note that since you are navigating backwards without an animation, the view
   * and therefore the modal will be unmounted immediately. As a result, the modal
   * will not have a dismissing animation. This is one of the reasons why it's best
   * to avoid coupling a modal with routing.
   */
  const router = useIonRouter();
  const closeDetails = () => {
    router.goBack();
  };

  return (
    <>
      <IonPage>
        <IonHeader>
          <IonToolbar>
            <IonTitle>Items list. Click on item to open details</IonTitle>
          </IonToolbar>
        </IonHeader>
        <IonContent>
          <IonList>
            <IonItem
              button
              routerLink={`/items/1/`}
              routerAnimation={animation}
              detail={true}
            >
              <IonLabel>Item 1</IonLabel>
            </IonItem>
            <IonItem
              button
              routerLink={`/items/2/`}
              routerAnimation={animation}
              detail={true}
            >
              <IonLabel>Item 2</IonLabel>
            </IonItem>
          </IonList>
        </IonContent>
      </IonPage>
      <IonModal
        isOpen={itemId !== undefined}
        onWillDismiss={closeDetails}
        initialBreakpoint={0.9}
        breakpoints={[0.9]}
      >
        <IonHeader>
          <IonToolbar>
            <IonButtons slot="start">
              <IonButton onClick={closeDetails}>Close</IonButton>
            </IonButtons>
            <IonTitle>Item {itemId} details</IonTitle>
          </IonToolbar>
        </IonHeader>
        <IonContent>
          <p>
            Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum
          </p>
        </IonContent>
      </IonModal>
    </>
  );
}

---

As I noted previously, the current behavior in Ionic is intended, so I am going to close this.

[legendar]

@liamdebeasi Thanks for the reply.

I understand why this is considered intended behavior. However, I can't agree that it should be the only behavior. The example with the modal is not very illustrative, but as I mentioned earlier, the issue is not specific to modals but affects anything that depends on routing changes.

Here is another example with content filtering. We have a custom filtering element at the top of the page, and underneath that, there's a list that filters based on the choices made in this element. See the screenshot of one of the filtering elements below:

When a user makes a choice, we want to display the same view and just filter the content. However, we also want to change the route so the user can share a link or open the page with a predefined value for this selector. Additionally, we want the swipe action to navigate to the previous view (any other page), but not just change the value in the selector. This is easy to achieve by disabling page animation and using the replace action on the route.
We also want to display a bubble with a hint for some choices, and we want the arrow of the bubble to be aligned with the selected choice. The issue here is that we can't use animation for the bubble arrow, as on every route change, a new view with a new bubble element and new arrow will be created.