bug: Opening card modal sets wrong color on statusbar if swipeToClose is set to true

[torgnywalin]

Prerequisites

• I have read the Contributing Guidelines.
• I agree to follow the Code of Conduct.
• I have searched for existing issues that already report this problem, without success.

Ionic Framework Version

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

Current Behavior

Opening a card modal with swipeToClose: true will change StatusBar style to the style of the device.

Expected Behavior

I would expect that the behavior is the same regardless of what swipeToClose is set to.
And that if the modal is to change the statusbar it should be able to set it back to what is was set to before.

If keeping track is not possible I think there should be options for:

• If the modal should be able to change the statusbar style at all (default to true for backward compability)
• What style to return to (default to default)

Steps to Reproduce

To reproduce:

• Open app on ios (ionic cap run ios)
• The statusbar style is dark as set in App.vue (Statusbar is white on white)
• Click the "Open card modal (with swipeToClose: false)" button
• The statusbar is visible when modal is visible (No changes to statusbar style)
• Click the "Open card modal (with swipeToClose: true)" button
• The statusbar is visible when modal is visible (Now the statusbar style is changed to light)
• Click the "Open card modal (with swipeToClose: false)" button
• The statusbar is not visible when modal is visible (No changes to statusbar style)

Code Reproduction URL

https://github.com/torgnywalin/issueModalStatusBar

Ionic Info

[WARN] Error loading @capacitor/android package.json: Error: Cannot find module '@capacitor/android/package'

       Require stack:
       - /Users/torgnywalin/.nvm/versions/node/v16.13.1/lib/node_modules/@ionic/cli/lib/project/index.js
       - /Users/torgnywalin/.nvm/versions/node/v16.13.1/lib/node_modules/@ionic/cli/lib/index.js
       - /Users/torgnywalin/.nvm/versions/node/v16.13.1/lib/node_modules/@ionic/cli/index.js
       - /Users/torgnywalin/.nvm/versions/node/v16.13.1/lib/node_modules/@ionic/cli/bin/ionic

Ionic:

   Ionic CLI       : 6.20.3 (/Users/torgnywalin/.nvm/versions/node/v16.13.1/lib/node_modules/@ionic/cli)
   Ionic Framework : @ionic/vue 6.3.3

Capacitor:

   Capacitor CLI      : 4.4.0
   @capacitor/android : not installed
   @capacitor/core    : 4.4.0
   @capacitor/ios     : 4.4.0

Utility:

   cordova-res : not installed globally
   native-run  : 1.7.1

System:

   NodeJS : v16.13.1 (/Users/torgnywalin/.nvm/versions/node/v16.13.1/bin/node)
   npm    : 8.1.2
   OS     : macOS

Additional Information

No response

[liamdebeasi]

Thanks for the report. Does this issue happen with canDismiss? The swipeToClose property is deprecated and should not be used.

[torgnywalin]

I see the same behavior if i set canDismiss to true.
The StatusBar changes color when card modal is displayed, and never reverts to previous state. (reverts to default for device)

I have made a new commit to reproduction repo with canDismiss instead of swipeToClose.

Step 1: App is started with StatusBar style set to DARK (Status bar is white on white)

Step 2: Opens modal - StatusBar is white on black

Step 3: Closes modal - Status bar is set to device style, not previous state

Documentation
I also think that the documentation needs to be updated to reflect that the swipeToClose property is deprecated.
It's mentioned somewhere on the top, but not at the properties level. I think most people would use this for reference, and not read the top each time.

[realityfilter]

Any good workaround or patch available in the meantime?

[realityfilter]

I use this patch with patch-package:

diff --git a/node_modules/@ionic/core/components/ion-modal.js b/node_modules/@ionic/core/components/ion-modal.js
index e360c89..029bc73 100644
--- a/node_modules/@ionic/core/components/ion-modal.js
+++ b/node_modules/@ionic/core/components/ion-modal.js
@@ -43,7 +43,7 @@ const StatusBar = {
     if (!engine) {
       return;
     }
-    engine.setStyle(options);
+    // engine.setStyle(options);
   },
 };

[sean-perkins]

Hello everyone, can you test with this dev-build and let me know if the problem persists? The dev-build caches the status bar style when the modal is presented and returns back to that value when the modal is dismissed.

6.3.7-dev.11668124677.10f80791

For the provided reproduction:

npm install @ionic/vue@6.3.7-dev.11668124677.10f80791

This is the behavior I am seeing in the dev-build:

Kapture.2022-11-10.at.19.05.48.mp4

Note: Changes are on the fix/status-bar-style branch

[realityfilter]

There is another problem with setting the status bar color in the first place. Take for example https://ionicframework.com/docs/api/modal#mounting-inner-contents
The modal opens full screen and is not stacked. That is how we use it. Ionic still sets the status bar color to light.

[sean-perkins]

@realityfilter can you provide a reproduction or more about your setup?

Ionic only sets the status bar style in very specific scenarios:

1. swipeToClose is enabled
2. canDismiss is not undefined and there is a presenting element (card style modal)
3. The modal is swiped to close and has reached the dismiss threshold for dismissing the overlay (depends on 1 or 2 being also true)

It shouldn't be applying for full screen modals without swipe to close.

[realityfilter]

We are using swipeToClose. Because modals with canDismiss enabled but without an presentingElement can not be closed via swiping.

[torgnywalin]

@sean-perkins Thanks, the dev build seems to solve our problem.
Hopefully it will be part of a release in the near future :)

When it comes to swipeToClose i think there is a difference to how canDismiss and swipeToClose works.

Is the idea that all modals with a presenting element should be swipable, and all fullscreen modals should not?
And if you want to disable swipeToClose on a modal with presentingElement you have to set canDismiss to false, and then set it to true on all @click events for dismiss buttons?
This it easy in examples with inline modals, but needs a bit more code when using modalController.

If this is the case, I agree with @realityfilter that there is a missing option to enable swipeToClose on fullscreen modals (if we are not to use swipeToClose anymore)

[sean-perkins]

@torgnywalin @realityfilter thank you both for your feedback around this issue.

This status bar style override really should only ever applied to card modals, but one of our checks was off. With some discussion with the team we have corrected this. Here's an updated dev-build to test with, that shouldn't apply the status bar style overrides when using a fullscreen modal:

6.3.7-dev.11668530465.1a2c771c

The intention of deprecating swipeToClose, is that only sheet modals and card style modals (modals presented with a presentingElement should support swiping to dismiss the overlay). Full screen modals should not. This decision was made in alignment with iOS design patterns, but if you have use cases and examples of applications that are doing this, I think a dedicated issue to that request would be encouraged.

Your feedback surrounding canDismiss and handling that in a conditional situation (e.g. cannot dismiss by default, but clicking a button allows dismissing) is valid and we should definitely discuss how to handle that with a simple Dx. I wonder if we passed the role to the canDismiss implementation, so you could do something like:

canDismiss = (role: string) => {
  return role === 'confirm' || role === 'cancel';
}

<ion-modal [canDismiss]="canDismiss"></ion-modal>

Optionally we could pass along data too, so you can make informed decisions based on the handlers you have on your button OR the parameters you pass when calling the dismiss API manually.

[sean-perkins]

@torgnywalin could you share the use case for wanting to disable swipe to close on a card modal? The API changes make sense to me, but I also want to tie that understanding to UI as well.

[realityfilter]

Thank you very much for the explanation and the effort. Some background info. We are using vue and the modal code is handled in composable functions that abstract away the underlying framework. It is hard to access the presentingElement outside of components. That is why we happen to use fullscreen modals in the first place. Could the presentingElement not be derived from the framework itself?

[torgnywalin]

@sean-perkins I'm not sure if I have any good use cases. I think it's been with us from earlier versions. Since we programaticly open the modal with modalController, it was easier to just create the modal with swipeToClose: false and then have control of closing the modal in the modal component. The button click event did the checks and dismissed or alerted about why it can't dismiss, but I agree that that is not a great solution.

So if role and even data is available in the canDismiss function that would work great for us.

Also I have no issues with not being able to swipe to close a fullscreen modal. Thanks for explaining your thoughts about why this was done.

[sean-perkins]

@realityfilter thanks for the added detail. In those decoupled functions, is it possible to query the DOM to look-up the element you expect to be the presenting element? For example, this could be the nearest ion-router-outlet. Controller overlays are historically disconnected from the DOM (they do not exist in the DOM until the controller creates the element, at the root of the DOM). So guessing which element would be difficult for Ionic to determine, since we do not know the context of which "page" or "element" that modal was opened from. Inline overlays are different, there could be some default behavior we could consider, since the element has a physical location in the DOM tree that could be used to determine a fairly good guess of what element is the presenting element.

@torgnywalin I've logged that feature change here, if you would like to track it: #26292. The team has discussed this change and we want to resolve this pain point before removing swipeToClose, so it will be in a minor release in the future.

I've also opened a PR to track the original issue with the status bar style here: #26291. That will be available in a weekly patch release, once the team has reviewed and approved it.

Appreciate the positive discussion here. If you have ideas or pain points that you can provide context on, would love to discuss 👍

[torgnywalin]

@realityfilter I think you can solve that by setting an id on ion-router-outlet as @sean-perkins describes.

<ion-router-outlet id="main"></ion-router-outlet>

let presentingElement;
const topModal = await modalController.getTop();

// We use topModal to stack modals if we open one modal from within another modal.
if (topModal) {
  presentingElement = topModal;
} else  {
  presentingElement = document.getElementById('main');
} 

const modal = await modalController.create({
   presentingElement: presentingElement,
   ... // Rest of properties
})

I have not tested this exact code, but we do something very close to this when opening modal in our apps.

@sean-perkins Thanks, would love to have a discussion about some ideas and pain points, but I would need to gather some more context around them, hope it's ok If I contact you at a later time

[realityfilter]

@torgnywalin Thanks. That helps. I will try this.

[ionitron-bot]

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.