nativescript-community / ui-material-components

Monorepo that contains all of the NativeScript Material Design plugins.
https://nativescript-community.github.io/ui-material-components/
Apache License 2.0
218 stars 80 forks source link

[BottomSheet] No content shown when used with RAD SideDrawer #462

Closed alexander-mai closed 6 months ago

alexander-mai commented 6 months ago

An Empty white BottomSheet is shown, when it is used in Combination with RAD SideDrawer as Root-Element.

Which platform(s) does your issue occur on?

Please, provide the following version numbers that your issue occurs with:

Please, tell us how to recreate the issue in as much detail as possible.

Create an app with nativescript-ui-sidedrawer as root element. An opened bottomSheet is only white without any content. On Android is the content of the bottomsheet correctly shown. The problem exists since ui-material-bottomsheet version 7.2.43

Is there any code involved?

farfromrefug commented 6 months ago

@alexander-mai sorry to hear that. If i find time i ll take a look, but i dont support RAD sidedrawer (too complex, too old, and not open source). I would advise to use @nativescript-community/ui-drawer

alexander-mai commented 6 months ago

Thank you @farfromrefug for your fast response. I would like to use @nativescript-community/ui-drawer, but it is not possible to use it as root element afaik. Has this changed in the last month?

farfromrefug commented 6 months ago

@alexander-mai not sure why it would not be possible. Do you have a link to an error ?

alexander-mai commented 6 months ago

I have tested it some years ago and there is also an Issue about the root problem: https://github.com/nativescript-community/ui-drawer/issues/3. I will give it a new try in the next days.

JWiseCoder commented 6 months ago

This issue was caused by https://github.com/nativescript-community/ui-material-components/commit/9a524e01a58cfda2a20800427c874f3cb5d96950

The problem appears to be the change in which view controller it's using to display the bottom sheet. In theory, it should be using the same one, provided it's not a nested bottom sheet. But instead currentView defaults to the root view, and the parentController becomes the view controller of that. But before, parentController was the nearest viewController up the view hierarchy.

@farfromrefug, is this not the case?

Edit: I can fix this issue for my app if I just comment out the this.parent = Application.getRootView(); override that's happening in _showNativeBottomSheet. This is overriding the NativeScript view's parent property. Can this not at least be put in an if so it only overrides it when it needs to for showing the nested bottom sheets? According to NS documentation, the parent property should be readonly.

farfromrefug commented 6 months ago

@JWiseCoder thanks for investigating this! Indeed could be the issue. I ll take a look next week

JWiseCoder commented 6 months ago

@farfromrefug - After some more testing, found it works on 7.2.11, but breaks on 7.2.12 (and thereafter). I think it was this commit where the parent was first being overridden: https://github.com/nativescript-community/ui-material-components/commit/ae4d6cc89fd6f5a13f8542be5e93777855605878

alexander-mai commented 6 months ago

In my opinion, the problem is caused by this change: https://github.com/nativescript-community/ui-material-components/commit/cbd325c082afb57912f212449c13ccdd2b0e0d53 I have no problem using version 7.2.42. With next version 7.2.43 the bottomsheet is broken.

If I understand it right, setting this.parent = Application.getRootView(); before this._setupAsRootView({}); sets the RadSideDrawer as this.parent. Before this change, the bottomsheet itself became the root view by this._setupAsRootView({}); and this.parent = Application.getRootView(); sets the bottomsheet itself as parent. Please correct me if I'm wrong.

farfromrefug commented 6 months ago

@alexander-mai could be related yes. So you understand the reason why i now set RootView as parent is for the bottom sheet to correctly inherit root css variables/classes Looking at it

farfromrefug commented 6 months ago

@alexander-mai @JWiseCoder should be fixed in latest!

alexander-mai commented 6 months ago

I have tested the changed from your pull request in my app with the current version of the bottomsheet and the RadSidedrawer as root element. It works perfectly.

Thank you very much @farfromrefug.

farfromrefug commented 6 months ago

@alexander-mai can we close this now?

alexander-mai commented 6 months ago

Yes, I think so.