Ionic-framework: feat: ios card style modal for iPad should not affect routerOutlet

Created on 5 Mar 2020  路  15Comments  路  Source: ionic-team/ionic-framework

Feature Request

Ionic version:


[x] 5.0.4

Describe the Feature Request

At the moment the card-style-modal only looks good on mobile view. On tablet view and especially with split-pane there is a lot of potential for improvement.
There should be an option to open a modal as usual. Subsequent nested modals should not have any influence on the routerOutlet, but only on the underlying modal

Current behaviour:

Link to video

Describe Preferred Solution

calling a child modal by using

presentingElement: await this.modalController.getTop()

should appear as in the picture by providing additional property like

  public async onOpenModal() {
    const modal = await this.modalController.create({
      component: TestPage,
      swipeToClose: true,
      presentingElement: await this.modalController.getTop(),
      style: 'tablet'
    });
    return await modal.present();
  }

furthermore nested modals should respect the given width/height provided by cssClass.

Additional Context

You can find this kind of card style modal in the IOS Appstore

core bug

Most helpful comment

@liamdebeasi so much wow!

I've tested your dev build using iPhone and iPad view on chrome browser.
Now --width and --height are applied to modals. Backdrop dismiss also works.
here are some working samples using 600px for --width and --height:

also i discovered a bug, where card style modal gets incorrectly applied on iPad using

--width: 100%;
--height: 100%;

heres the sample:

thank you very much for your high commitment!

Update

pushing modals on android tablet view leads to overlapping backdrop background color.
Sample:

Update 2

I've deployed it on my device. Seems like the modal wont show the stacked card view, instead taking full height.
Modal is using 600px for width and height for cssClass (also without cssClass attribute, it makes no difference).
Sample:

All 15 comments

Thanks for the issue. In the case of a split view application, you should be targeting ion-split-view, not ion-router-outlet since that will not account for the side menu. Can you try targeting that instead and see if the issue persists?

Thanks for the issue. In the case of a split view application, you should be targeting ion-split-view, not ion-router-outlet since that will not account for the side menu. Can you try targeting that instead and see if the issue persists?

Could you provide a snipped on how to target ion-split-view instead of ion-router-outlet for creating a card style modal?
I cannot find any IonSplitView, but only IonSplitPane.
I can't find the property nativeEl on IonSplitPane.

Sorry, I mean ion-split-pane. You can just use a document.querySelector or use Angular's ViewChild to select it.

Sorry, I mean ion-split-pane. You can just use a document.querySelector or use Angular's ViewChild to select it.

Yes, now also the ion-split-pane get affected. Thank you.
Following code was used:

  public async onOpenModal() {
    const modal = await this.modalController.create({
      component: TestPage,
      swipeToClose: true,
      presentingElement: document.querySelector('ion-split-pane')
    });
    return await modal.present();
  }

But this was not the intention of my feature request.
Sorry for misleading feature title, i'll try to explain by pictures:

Current behaviour using card style modal:

Wanted behaviour using card style modal:

as you can see, the second level modal is not taking the full width, but respects width and height of parent modal.

for comparison, this is what you get if you:

  1. create regular modal (without presentingElement)
  2. inside that modal call another modal using modalController.getTop() as presentingElement

Thanks for the clarification! I am going to change this to a bug since Ionic's card style modal does not follow the spec for iPad, but it should.

@liamdebeasi i tried your dev build dev.202003111951.2b4c675.

It works just like I thought it would!
But I have discovered a few little things:

  • property backdropDismiss: true is not working
  • property cssClass: 'default-modal-size' wont apply height and width

    • by default the modal wont take size of normal modal (600x600 px)

default-modal class in global.scss:

.default-modal-size {
    --width: 600px;
    --height: 600px;
}

used code for parent modal:

  public async onOpenModal() {
    const modal = await this.modalController.create({
      component: SettingsPage,
      cssClass: 'default-modal',
      backdropDismiss: true,
      swipeToClose: true,
      presentingElement: this.routerOutlet.nativeEl
    });
    return await modal.present();
  }

used code for child modal:

  • side question: assumed cssClass would work on the parent modal. Will the child modal automatically take its size?
  public async openModal() {
    const modal = await this.modalController.create({
      component: SettingsPage,
      backdropDismiss: true,
      swipeToClose: true,
      presentingElement: await this.modalController.getTop()
    });
    return await modal.present();
  }

Furthermore i think, there should still be the option to call fullscreen modals. For example, you can find these in the mail app. Heres a screenshot:

also heres a link to a video of current behavior of your dev build:
For comparison you can see the normal modal too.
Link to video

Thanks for the useful feedback!

  1. The backdrop dismiss is intentional. There does not seems to be a backdrop dismiss in a native modal on iPadOS when I tested in a native app. (I will double check on this, but I am pretty sure it is supposed to be this way)

  2. The card modal sets its own width/height via CSS variables, so I will need to make sure users can overrides these.

  3. The child modal will not taken on the size of the parent modal -- they are two separate instances. This is consistent with how other modals work in Ionic.

  4. We can investigate doing a fullscreen modal, but this seems to be separate from the card style modal so it might be best to create a separate feature request.

I will work on these changes and will post another dev build for testing. Thank you!

Thanks for the useful feedback!

  1. The backdrop dismiss is intentional. There does not seems to be a backdrop dismiss in a native modal on iPadOS when I tested in a native app. (I will double check on this, but I am pretty sure it is supposed to be this way)
  2. The card modal sets its own width/height via CSS variables, so I will need to make sure users can overrides these.
  3. The child modal will not taken on the size of the parent modal -- they are two separate instances. This is consistent with how other modals work in Ionic.
  4. We can investigate doing a fullscreen modal, but this seems to be separate from the card style modal so it might be best to create a separate feature request.

I will work on these changes and will post another dev build for testing. Thank you!

to your point 4:

  • the previous behavior is already the wanted feature, that should still be usable (except the routerOutlet there is affected).

    • #20255

looking forward for next dev-build!

Thanks for the follow up. The behavior shown in the Mail app appears to be something separate than the card modal that you showed in https://github.com/ionic-team/ionic/issues/20700#issuecomment-595666870. I created a separate feature request for that here.

Ok I fixed a couple things:

  1. I restored backdrop dismiss functionality. Native iOS seems to not use it on iPadOS with the card modals, but it is a breaking change for Ionic Framework so I decided to leave it in.
  2. You should be able to override the dimensions of the modal now.

Can you try the following dev build and let me know if it resolves the issue?

npm i @ionic/[email protected]

@liamdebeasi so much wow!

I've tested your dev build using iPhone and iPad view on chrome browser.
Now --width and --height are applied to modals. Backdrop dismiss also works.
here are some working samples using 600px for --width and --height:

also i discovered a bug, where card style modal gets incorrectly applied on iPad using

--width: 100%;
--height: 100%;

heres the sample:

thank you very much for your high commitment!

Update

pushing modals on android tablet view leads to overlapping backdrop background color.
Sample:

Update 2

I've deployed it on my device. Seems like the modal wont show the stacked card view, instead taking full height.
Modal is using 600px for width and height for cssClass (also without cssClass attribute, it makes no difference).
Sample:

Thanks for the follow up.

also i discovered a bug, where card style modal gets incorrectly applied on iPad using

--width: 100%;
--height: 100%;

This is the correct behavior. 100% is going to make the dimensions of the modal be 100% of the parent container. In this case, that parent element is ion-app. If you want some offsets built in, you will need to add them yourself when specifying --width and --height.

pushing modals on android tablet view leads to overlapping backdrop background color.

If you feel this is a bug please open a separate issue as this does not appear to be related to the card modal updates.

I've deployed it on my device. Seems like the modal wont show the stacked card view, instead taking full height.

Fixed! Here's a dev build 5.1.0-dev.202003171608.9d9a02f.

This PR looks to be in pretty good shape, so I will get this reviewed and merged in as soon as I can. Thanks for all the testing and feedback!

I am going to keep this issue open until I can get the PR merged in. Thanks!

Thanks for the issue (and for all the testing and feedback!). This has been resolved via https://github.com/ionic-team/ionic/pull/20750 and will be available in an upcoming release of Ionic Framework.

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

alan-agius4 picture alan-agius4  路  3Comments

gio82 picture gio82  路  3Comments

brandyscarney picture brandyscarney  路  3Comments

vswarte picture vswarte  路  3Comments

daveshirman picture daveshirman  路  3Comments