fix(modal): changing the open property no longer calls beforeClose

[driskull]

Related Issue: #6407 #6379

Summary

• Changing the property open will no longer call beforeClose.
  • Its up to the developer to call beforeClose when modifying the property.
  • This ensures that the classes added by modal will always be cleaned up.
• User interaction such as clicking the close button will call beforeClose
• Cleanup

[driskull]

@jcfranco I heard back from Stencil regarding validation in the watcher. Here is their response:

Using a watcher to validate and potentially revert property changes based on certain conditions is a valid approach in Ionic apps. The code snippet you provided demonstrates the usage of a watcher to observe changes to the myProperty and perform validation logic.

Based on the information you have provided the approach you described using a watcher to validate and potentially revert property changes is acceptable and aligns with the purpose of watchers in Ionic apps.
Thank you for using ionic,

So we can revert changes in the watcher. However, since our beforeClose property is async, there is a delay before the property is reverted which leads to unwanted animation changes.

If the beforeClose takes any significant time it will basically close the modal and reopen it. The shifting is even noticeable with a function that just returns a resolved promise.

I think I lean towards not having the watcher flip the property for this reason. What do you think? If we don't flip the property, users will just have to call beforeClose before toggling the property.

[benelan]

Code LGTM, but some people may see this as a breaking change since they may be relying on this behavior. I'll wait for Franco for final review due to that.

Nitpick: can you change "should" to "does" in the PR title. Or: "... property no longer calls beforeClose"

[driskull]

Something else that we could do instead of this would be to introduce an internal state that handles the open prop as well as the beforeClose. So if a user sets the open prop to false it won't actually close until both the open prop and beforeClose have resolved. That would eliminate any temporary style changes. We would likely need to update our CSS to not use the open attribute and use a class instead.

@jcfranco is this what you want to pursue?

[github-actions]

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

[driskull]

Ping @jcfranco