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

bug: setting ion-modal width or height CSS properties to auto results in size of 0 #24080

Closed
5 of 6 tasks
Julien-Marcou opened this issue Oct 15, 2021 · 10 comments · Fixed by #25630
Closed
5 of 6 tasks
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@Julien-Marcou
Copy link

Julien-Marcou commented Oct 15, 2021

Prerequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x

Current Behavior

According to Ionic documentations, when creating an alert with complex form, we should use a modal instead

If you require a complex form UI which doesn't fit within the guidelines of an alert then we recommend building the form within a modal instead.

I'm trying to create a simple alert which prompt the user to confirm his password before validating critical actions, but I can't add form validations on a <ion-alert>, so I switched to a <ion-modal> on which I try to replicate the design of an <ion-alert> using CSS properties (--width, --height, ...) so the modal doesn't take the entire app's viewport but appears floating on top of the current page, but setting them completely break the modal, which I don't think is expected

Expected Behavior

Using the CSS properties defined by Ionic: --width, --height, --min-width, --min-height, --max-width & --max-height, I should be able to create a floating modal on top of the current content

If the <ion-modal> isn't expected to be used to create floating modal, then all these CSS properties as well as --backdrop-opacity, --border-radius & --box-shadow are useless

Steps to Reproduce

Inside the component which create the modal:

const modal = await this.modalController.create({
  component: ConfirmPasswordModalComponent,
  cssClass: 'confirm-password-modal',
});
await modal.present();

Inside the global.scss file:

.confirm-password-modal {
  --width: auto;
  --height: auto;
  --min-width: 280px;
  --max-width: 320px;
  --min-height: auto;
  --max-height: 90%;
  --overflow: auto;
  --border-radius: 4px;
  --box-shadow: 0 11px 15px -7px rgba(0, 0, 0, 0.2), 0 24px 38px 3px rgba(0, 0, 0, 0.14), 0 9px 46px 8px rgba(0, 0, 0, 0.12);

  .modal-wrapper {
    margin: 12px;
  }
}

Code Reproduction URL

No response

Ionic Info

@ionic/angular: 5.8.4
@ionic/cli: 6.17.1
@angular/cli: 12.2.10
@capacitor/cli: 3.2.5
NodeJS: 16.11.1
npm: 8.0.0

Additional Information

Here is a video demonstrating the issue on Firefox & Chrome and what the problem is:

https://youtu.be/JvJQbTfKsrw

As you can see I'm able to fix the issue doing some manipulations on the Component inside the .ion-wrapper:

On Firefox, adding these CSS rules display: block; & position: static; is enought to fix the problem, but on Chrome I also have to add the rule contain: initial;

A quicker fix would be to be able to remove the ion-page class that is automagically added to my Component, as all the 3 CSS rules I added are here to override the ones defined by .ion-page

I opened this as a bug as the ion-page class makes it impossible to resize the modal using the CSS properties defined by the <ion-modal> without overriding the .ion-page CSS rules, but it can also be seen as a feature request, by adding an option to control whether or not we want the ion-page class to be automagically added to our component, for example, when creating the modal:

const modal = await this.modalController.create({
  component: ConfirmPasswordModalComponent,
  cssClass: 'confirm-password-modal',
  componentClass: '', // Defaults to 'ion-page' but can be overrided with whatever we want
});
await modal.present();
@ionitron-bot ionitron-bot bot added the triage label Oct 15, 2021
@liamdebeasi
Copy link
Contributor

Thanks for the issue. I can reproduce this behavior, but I need to spend more time digging into the source of the issue. I will follow up here when I have more to share.

@averyjohnston
Copy link
Contributor

Hi, I've dug into this and confirmed that the ion-modal implementation could be improved here. The issue is that setting --width and/or --height to auto leads to a size of 0px. Your code has a defined min-width, but min-height is also set to auto, so the modal's size is 280px by 0.

It looks like the cause is that both the outer ion-modal and inner .ion-page are absolutely positioned, which messes with the height calculation.

In the meantime, another possible workaround would be to set --min-height to something besides auto, though I acknowledge this isn't perfect since the modal wouldn't just fit its content.

@averyjohnston averyjohnston changed the title bug/feat: Using CSS properties to create a floating ion-modal result in invisible modal bug: setting ion-modal width or height CSS properties to auto results in size of 0 Nov 10, 2021
@averyjohnston averyjohnston added package: core @ionic/core package type: bug a confirmed bug report labels Nov 10, 2021
@ionitron-bot ionitron-bot bot removed the triage label Nov 10, 2021
@t0byman
Copy link

t0byman commented Dec 10, 2021

In march of 2019 sonichk provided a fix for this in https://forum.ionicframework.com/t/dynamic-modal-height-based-on-content-in-ionic-v4/139595/9 but then you would have to replace the ion-content in your modal with a <div class="modal-content"> or something making you lose the ability to do scroll events on that ion-content. So I made a fix where you can keep ion-content in your modal and still have it be auto height, in your modal.vue just do:

<style lang="scss" scoped>
ion-content {
  display: contents;

  &::part(scroll) {
    position: static;
  }
}
</style>

<style lang="scss">
// Keep non-scoped
ion-modal {
  --height: auto !important;

  .ion-page {
    max-height: calc(100vh - (var(--ion-grid-padding) * 2));
    position: relative;
    contain: content;
  }
}
</style>

@Julien-Marcou
Copy link
Author

Julien-Marcou commented Dec 29, 2021

Since Ionic v6, you can no longer target the direct .ion-page child as we used to, as we can't access the component injected into the modal from the modal wrapper itself (because of the shadow DOM).

So you can no longer do:

/* global.scss */
ion-modal {
  --width: auto;
  --height: auto;
  --max-width: 90%;
  --max-height: 90%;
  --overflow: auto;

  .modal-wrapper > .ion-page {
    position: static;
    contain: initial;
  }
}

But thanks to the shadow DOM, the injected component is now seen as the direct child of the modal, which allow us to target it directly without using the wrapper.

Here is what I came up with:

/* global.scss */
ion-modal {
  --width: auto;
  --height: auto;
  --max-width: 90%;
  --max-height: 90%;
  --overflow: auto;

  > .ion-page {
    position: static;
    contain: initial;
  }
}

You can put whatever max width & max height you want, but you have to set them in order for the modal to be scrollable instead of it overflowing the size of your device's screen if you have to much content inside of it.

I recommend only targeting the direct .ion-page child to avoid side effects if you end up with nested .ion-page.

As for the position: static; vs position: relative;, the only advantage of the relative position is so that you can make use of the z-index CSS rule, but it's not strictly necessary as the modal is placed over the rest of your content by default and is centered using CSS flex rules (your mileage may vary if you have other absolute element in your UI).

And finally for the contain CSS rule, it's a performance optimization rule only supported by Chromium browsers at the moment.
Ionic sets it to layout size style by default, but in order for it to work properly, the browser need to know the size of the .ion-page, which it cannot in our case, so you can either fix it by specifying the contain-intrinsic-size (but then our modal is no longer responsive), or you can add paint to the list of contain optimizations, at that point you end up with contain: layout size style paint;, but I prefer to use contain: initial;, that way I'm sure it is rendered the same way on all browsers.
I don't know if using contain: content; (which is equivalent to contain: layout paint;) can end up breaking something, are you are essentially removing the size & style keywords that have been added by Ionic.

@Webrow
Copy link

Webrow commented Feb 24, 2022

This is still an issue. The three options of a modal are "to different" from what a modal was originally, so we also modified the style of it. (basically a floating card) using

ion-modal {
  --max-height: 80%;
  --max-width: 90%;
  --background: transparent;
  --border-radius: 5px;
  --height: auto;
}

No problems while testing on the browser, devices won't show the modal nevertheless.

@liamdebeasi
Copy link
Contributor

Hi everyone,

I have a PR up that should improve our compatibility with custom dialogs: #25630

Here is a dev build if anyone is interested in testing: 6.1.15-dev.11657743655.1e3dcc3c

A few notes about this change:

  1. If you want to have both the height and width of the modal be automatically sized based on the size of the inner content, you must not use ion-content. The reason for this is ion-content is designed to be automatically sized based on the dimensions of the parent. If the dimensions of the parent are based on the inner content (I.e. height: auto), but the dimensions of the inner content are not known, the modal will have a height of 0. (This is the issue that was reported)
  2. Using ion-header or ion-footer without an ion-content will not work as expected and is not recommended.

Here is an example CodePen: https://codepen.io/liamdebeasi/pen/BarLXee

@Julien-Marcou
Copy link
Author

I can confirm that it fixes the issue 🚀

@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #25630, and a fix will be available in an upcoming release of Ionic Framework.

@liamdebeasi
Copy link
Contributor

When this ships we will also publish new documentation here: https://ionicframework.com/docs/api/modal

@ionitron-bot
Copy link

ionitron-bot bot commented Aug 14, 2022

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.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Aug 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants