-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Modal Page with transparent background #6558
Modal Page with transparent background #6558
Conversation
We will need to make some work for IOS13 i wonder if we can do this at that time. |
7e98dd5
to
e699051
Compare
Rebased against 4.1.0 |
I don t think we can put this on 4.1 as we are cutting a stable. but we will run tests now and see |
/azp run |
No pipelines are associated with this pull request. |
@ahoefling @hartez is this something we want to have next to #1778? Effectively we would then have two ways of showing popups which might be confusing. Don't get me wrong @andreinitescu I very much appreciate the effort! Just thinking out loud. Also, I think there are a couple of breaking changes in this PR. Here you change the default value to this new way of showing popups, potentially breaking the current implementation for people. And here you change the default background color to clear instead of white. Although it's just a styling issue people might see different visuals popping up without them changing anything. I know you mentioned these in your PR already, I think we just need to be sure on this. |
@andreinitescu this is a great PR and works well as far as I can tell! The thing that makes #1778 very difficult, is it defines the APIs that must be used natively which would encapsulate a This PR provides a partial implementation for #1778 (which is identified in the description) as it doesn't implement every part of the spec, which is identified as a partial completion in the PR description. I am in agreement with @jfversluis and I don't think we should have 2 competing specs or even a partial spec we refine later. Once we ship a new feature we can't make any large breaking changes. If we accept this as a stopgap feature for #1778 and when that feature is fully implemented it will break many apps out there. Providing a migration path is not really a great option either. All things I considered so we could ship this code and then swap it out with the final implemenation of #1778 when it is ready. As far as #1778 is concerned, I have been doing a lot of work on it and have a working implementation for Android and UWP, I have not had time to circle back for the iOS implementation and plan to get to it later this summer. |
While I mentioned about popups and #1778, this is not meant as a fix for #1778. I mentioned it in the original description (I edited it, you need to see the previous edited version). It clearly does not come with any new popup API, it's still only about the modal. I edited the description to remove references to #1778 and popups if that helps. I don't understand why would it be confusing. It's not meant to tell developers "here's how you should do popups", #1778 is for that, it has a specific and richer API (ability to have buttons). I also mentioned in the description why I think it should not break existing apps. If I miss anything, please let me know exactly how is it breaking. The modal background is not visible at all because the modal wrapper is always completely filled by the page and the page has its own default color. Again, if I miss something let me know what is it exactly and I can try fix it. Maybe there's another way. Also, my PR and tweet is not trying to steal or put in shadow @ahoefling 's effort on #1778, it's a great effort and I'm looking forward to see that ticket finally implemented . I actually responded to him on Twitter that my PR and his PR are different things. |
Like I mentioned in the original description, this indirectly gives a very raw way to create popups. And I think there's still useful even with #1778, in the case someone wants a very flexible way to show popups. (Maybe I should not have edited the description, I described this extensively how you could do custom animations and positioning and whatnot. Well, the description is still visible if you click on the "edited" button and see the previous version). |
This is some really great work you did here! I think the messaging was confusing as it mentioned the Popup spec but as you (@andreinitescu) have mentioned it doesn't really implement it. I see in the files changed you updated the enum to allow for a transparent background and setting the background color. Then in the platform specific code you implement the colors being configured as the background color. This effectively gives us the transparent effect you have in the screenshots. It looks like the API does not change for displaying a modal screen, correct? NavigationPage.ShowModalAsync(new SomePage()); This may still cause a breaking change for anyone that has already entered a I still have some concerns about this enhancement being merged. I can be convinced otherwise. Merging this and #1778 would effectively create 2 ways to render the exact same popup/modal, granted the Popup Spec in #1778 has callbacks and this does not. @andreinitescu |
Don't worry @andreinitescu ! Nobody is saying or thinking this in any way! I understand these are two different things, but in the end, effectively they achieve the same thing. That was the only reason I am thinking: should we end up with two ways of doing the same thing? |
Thanks Gerald, I'm happy to discuss about concrete things. Here's why I think this PR is still worth it:
Let me know if you have more thoughts, I'm happy to discuss. It's obviously up to you (the XF team) to decide. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iOS13
The effect of this on iOS 13 is interesting. Currently iOS13 completely breaks Modals on XF because they now default to PageSheet and when a user swipes away a PageSheet it leaves the modal in a weird state. This PR effectively forces the behavior to a specific value instead of being Automatic.
I think this is fine for the scope of this PR but we'll want a follow up PR to support PageSheet and possibly leave the Modal Behavior as Automatic once we have PageSheet support
Popup Messaging
A different title for this PR could be "Enable the ability for users to specify OverCurrentContext for the UIModalPresentationStyle" and then just forget that anyone mentioned popup :-) if people want to use this for popups that's on them but we're not going to publish any official messaging that this is the way to go
What's broken with OverCurrentContext
Using OverCurrentContext breaks OnAppearing/Disappearing for the underlying page since the ViewController for the page is no longer dismissed. OverCurrentContext effectively makes this all work more like Android. The Android implementation for OnAppearing/OnDisappering is just inside the PusModal/PopModal calls on the Platform
CurrentPageController?.SendDisappearing(); |
So this PR will need to do something similar
What to use for the default
Hmmmmmm. It feels like we should focus this PR more around its new proposed title of "Enable the ability for users to specify OverCurrentContext for the UIModalPresentationStyle" and just make it so that type of PresentationStyle is functional instead of replacing default behavior. The argument for making this default would be that it makes the Android/iOS behaviors symmetrical. Which we have used to sway us into breaking changes in the past. @samhouts thoughts?
I will continue to check in on this. I'd much rather use this than install a nuget package to my projects. |
Oh please add this to next release of XF!! |
@andreinitescu can you rebase and address changes? Or do you want us to take this one over? |
It will be very helpful for us to get this feature in 4.3 |
@PureWeen any update on when we can expect this feature to be added? |
@davidortinau @PureWeen @andreinitescu @samhouts Thanks again. If any help needed to get this PR done, I am willing to help. |
@sasivishnu Thanks for asking! I'm afraid it's unlikely that this will make it into 4.4. We are still looking into this. I think we are ready to take over this one ourselves to get it in. Unless @PureWeen thinks otherwise. Actually, I have done some work on this now. Let's see if I can bring this home :) |
So not in 4.4, will it be 4.5? |
@XXcharlosXX we will do our absolute best to get it in 4.5, stay tuned |
fingers crossed! |
hoping y'all are able to squeeze this into 4.5!! |
Is this live? |
@XXcharlosXX looks like it was moved over to #8551 |
@samhouts , is there any documentation, how to use this. I can't see any transparent modal pages in v4.6 |
I don't think there is yet... @davidbritch do you know anything about that? |
@dinesh4official
|
@samhouts @andreinitescu @rmarinho , Unfortunately, Transparent Modal background is not working while we push the modal page from tabbed page. I've attached the sample to replicate the issue with latest XF version. Replication Procedure: Sample : |
@dinesh4official |
@AlleSchonWeg , yes i tried with that option and the issue still replicates. @samhouts , one more scenario to replicate the issue. Navigation.PushModalAsync(new NavigationPage(new TransparentPage()); |
We had the same problem with ios. Not sure if our ios dev has solved it. I thought he changes the ModalPresentationStyle. |
@AlleSchonWeg , thanks... |
@AlleSchonWeg , I've achieved the requirement through custom renderer and the content page appears with semi transparent background smoothly. Thanks :) |
Make sure you apply the presentation style & transparent background to the navigation page as well. It works for me. |
@brepetti , Good to hear that its working for you. But unfortunately, not for me. Anyhow Xamarin team has verified and logged as a bug report for this. And I've adapted work around for this until then which I've mentioned in the above link. Hope it may help someone who is link for alternate solution. Thanks :) |
Description of Change
This PR changes the background color of the modal container of a modal page, from white to transparent. This way, if the modal page has the background color with transparency, the content behind the modal page is now visible. In other words, this PR makes the opacity of the
BackgroundColor
on a Page really work (every Xamarin.Forms developer must have tried this and discovered it doesn't work ?? ).With this change, there's now a very easy way to finally be able to create popup pages, and the nice thing is it can be done using the existing simple modal navigation API.
Creating a popup page only requires a
ContentPage
with a semi-transparentBackgroundColor
, and some content not covering the whole page (the content position can be customized easily).Show the page as a modal page:
Closing the page is done using existing API:
On Android, since the "popup" is a simple modal page, the back button just works.
Once this is implemented, more features can be added, for example popping the modal page automatically when tapping anywhere on the page except its content.
Don't animate the overlay, only the content
If someone doesn't want to have the overlay animated but only the Page content, this can be achieved relatively easily by calling
Navigation.PushModalAsync(modalPage, animation: false)
and animating the page content.Animate the overlay and/or content
Similar to the previous, with
Navigation.PushModalAsync(modalPage, animation: false)
this can be done by:BackgroundColor="Transparent"
BoxView
with a semi transparentBackgroundColor=#A0000000
BoxView
overlay (animate the opacity, animate scaling it, etc.)Popups are a very common feature, so if possible it would be great to get this feature sooner.
API Changes
The only public API change is on iOS, where I added a new value for the UIModalPresentationStyle platform specific enum. This is needed because the default presentation style for the modal container changed from
UIModalPresentationStyle.FullScreen
toUIModalPresentationStyle.OverCurrentContext
in order to support a transparent background.AFAIK, this change should not have any backward compatibility implications, since for the page modal container, the OverCurrentContext value behaves exactly like the previous value, FullScreen.
Issue fixed
This doesn't (completely) fix the #1778, but I think it opens up new possibilities with very little changes.
Platforms Affected
Behavioral/Visual Changes
None. If you don't set semi-transparent background, there's no visual change. This is because all pages have the white color as background color by default.
Screenshots
Testing Procedure
See the "Modal With Transparent Bkgnd Gallery - Legacy" page.
PR Checklist