Supported Sheet Component #787
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Copied the structure from the bottom sheet dialog view
Used it from Dialog as well as NavigationStack. Need to created the stack nested in dialog in a whole different fragment manager - one not hosted by activity but custom hosted so can provide a different onBackPressDispatcher. Need to use the ComponentDailog's dispatcher so that the fragment manager can get the gesture back before the dialog gets it
There is no android.R.id.content in react native. So just added fragment without a container and it does show
Did it without the nasty shadow node stuff! Used layout params match_parent so it the root view gets all the space then firing update size is all that's needed. Had to remove the top, left, right, bottom of 0 from the Dialog style otherwise it wouldn't get bigger than it's original size (so it had the tabbar-sized gap at bottom on the twitter sample). Only need position absolute
Its the same code. Was only using the custom code because it was [recommended by android](https://developer.android.com/develop/ui/views/components/dialogs#FullscreenDialog). But couldn't use the android.R.id.content because it doesn't exist in react native and also the animation didn't work - not sure if that's because there's no content view? without the content view think it goes in separate window so transition doesn't work?!
The onAfterUpdateTransaction of the NavigationStack was called before the one in DialogView. Didn't think this could happen - remember testing this when doing original predictive back PR - had tabs inside tabs inside tabs etc. If the NavigationStack can't find the ancestor fragment then return early. It will get rerun later, when attached to window, for example. Had to add the touch stuff to DialogRootView otherwise couldn't interact with the stack. Copied the code from the BottomSheetDialogView
Can set the first child with a transparent border if want the scene behind to show through <View style={{flex: 1, borderColor: 'transparent', borderWidth: 15}} collapsable={false}>
Implemented a custom FragmentHostCallback so can get the FragmentManager to use the ComponentDialog's onBackPressedDispatcher. Without this, it uses the Activity's onBackPressedDispatcher - the Dialog's one always take precedence so the modal closes on the gesture back instead of moving back through the stack. Had to do a lot of debugging to work out what to implement in the FragmentHostCallback. Dispatching the start ensures the StackFragment is added (from the NavigationStack) - otherwise can't get a childFragmentManager from it. Cloning the layout inflater in onGetLayoutInflater ensures that SceneFragment doesn't get the same layout inflater as the StackFragment - otherwise get an error saying factory already set on LayoutInflator. Got this clone inflator code from the FragmentActivity - the native FragmentHostCallback
Followed the deprecation advice. Tried to use DialogView.this.getHandler but it was null so went with the main looper
Not sure if it's needed but plan to try and test. Also changed to show instead of showNow - couldn't tell if that adds the animation to the dialog display. But show still happens before the onAttached of the NavigationStackView so show is better
There's no need to put it in the ancestor stack. It doesn't use the fragment back stack to go back so doesn't need to be primary navigation. Clearer to put it in the top level support fragment manager. It starts a brand new support fragment manager of its own so makes sense to start a new ancestor stack with the dialog
The normal case is not the dialog so put it in the else
Made the open prop controlled
The DialogView isn't rendered when it's dismissed so no need to reset the flag. The screen is blocked until it's removed as well because the DialogView is rendered in front of scene (even when dialog not shown) - so no chance that the view could still be in hierarchy if open toggled off then on really quickly
There's only open and closed so no need to check event count. Only need event count if native can be out of step with js - need at least 3 values for that. Plus the view doesn't even render when open is false
For example, when tap r to reload with dialog open the dialog would stay open. Or when navigating back and dialog opens on last scene then would get blank dialog on current scene. Copied the code from bottom sheet dialog
This is like the [full screen recommendation from android](https://developer.android.com/develop/ui/views/components/dialogs#FullscreenDialog). Show the dialog fragment as a standard fragment. Can't add it to the android.R.id.content as per the recommendation because the content is too high up the view hierarchy for the fragment container to find it. Adding it to the support fragment manager would fix that but then the back gesture wouldn't work - the support fragment's handler is registered first so the main stack would take the gesture back instead of the support - so the dialog wouldn't close. I like this compromise. Plus it seems in keeping with the non-modal bottom sheet - that doesn't take up the whole view neither. Tried this before and couldn't get it to work - the trick was to add the dialog fragment to the same fragment manager - not the child. With the child got error saying already executing transaction when the modal stack tried to add fragment with commitAllowingStateLoss. Think it's ok to use the same fragment because the stack is actually A -> B -> Sheet A -> C - in this example, open the sheet and then click button to go to C in the scene behind the sheet (because it's non-modal). Had to check the scene in onBackStackChangeStarted because it can now be the SheetViewFragment when the gesture closes the dialog.
Plan is to have single sheet component so want them to have the same stack context. Not sure this works for bottom sheet because it doesn't use a fragment at all - but that can't have a stack then? maybe it needs a fragment?
No need to dynamically create it on the native side and then resize the first child. Can add it from js and resize itself. Have seen that sometimes the resizing isn't happening? don't know why but the content sometimes doesn't show?!
Not sure what it does so removing for now
The onAfter fires after props set but before children added. So if the dialog opens before the root is set then the dialog is blank. Usually the child wins so the dialog is there. Added check to ensure child always there first
Component works in modal and non-modal
If don't stop then fragment state manager tries to call the getOnBackPressedDispatcher and onGetLayoutInflater - but there's no dialog so get error. There's a bug with the Dialog where the back listener isn't cleared but think that's fixed in later android. Doesn't open the non-modal sheet but haven't investigated why yet
It tried to add fragment to container that wasn't in the window yet. Ensured it's been attached to window before trying to add the fragment
Makes it consistent with other sheets (like non-modal non-bottom). The default predictive back didn't work - on completion the sheet was hidden but then reappeared?! Tried requestLayout on CoordinatorLayoutView and that almost worked but still was a flicker. So forced it down the non-hideable path because that called setState instead of setStateInternal - this smoothed out the animation. Then set state to hidden if it was hideable. Had to add onBackPressedCallback to the right dispatcher - activity if not inside dialog and componentdialog's if inside dialog. A -> B (C -> D) where C is dialog and D is scene in dialog with bottom sheet.
Open a sheet and then changed language and returned to app. Then saw the different errors and added defensive checks. The unexpected one was that NavigationStack fragment wasn't added anymore so getting parent fragment manager threw error onDetached
Putting a modal inside a sheet (bottom sheet from before) didn't work - there was a gap before it showed. This is super weird because tried it in react native modal as there's no gap. Removing the animation (even though there isn't one for first scene) removed the gap - the stack showed straightaway. But no idea why it worked for react native modal where the animate flag was obviously the same - tried to find a difference in react native code but couldn't. Anyway, happy with the change because don't want to animated if 'first' scene (could be second scene if fluently navigating but still effectively the first if there aren't any previous.
The stacks have different NavigationContext if there's stacks inside them - want to use the top level NavigationContext only because want to run the code when the main scene is active again. So used a SheetContext with onNavigated - it's only useNavigated for the root one and then the child ones are called iteratively from the top down. A scene/sheet can only open one sheet so ok to have single onNavigated. Also turned off dismissViewControllerAnimated for now because this fails for similar reason. It's not just called for the top level scene/sheet anymore - opening a child sheet is also from a scene if it's from a stack inside the modal. Plan to use the root on the SheetContext for this - only dismissViewControllerAnimated if the viewControllerToPresent is root (can cast to sheet and check)
This prevents the problem of opening a child sheet with a modal stack closing all sheets. Had trouble remembering why this method was needed but then read commit 8f0f022 and finally remembered the timing issues. This method's needed when navigate from A with sheets open to B with a sheet open immediately. Without this dismiss with completion then the sheets on A will close but the sheet on B won't open
Wrapped the sheet specific (ios 15+) code in available blocks and tested the modal sheet shows up on ios 14
It's onNavigated handled by the child - maybe onNavigatedTop would be good name, too?
Using overfull instead of full because it supports transparency
Created basic files. Don't need resizing for NVSheet so not interfaceOnly and no cpp - think this is right but haven't tested yet
Created and registered the SheetViewManager
Created and registered the DialogViewManager
In twitter example, opened modal containing stack starting on notifications scene on fabric - not sure if this happened on non-fabric. About 1 out of 3 times the wrong tab would show - 'All' was active but mentions content was showing. Not sure why but setting the current item seems to fix it. Well, i did see it about 1 time out of 100. About 2 times out of 100 can see the other scene behind but can fix that by setting background color of the tab content
Fabric needs these files for a compile (can probably mark components as android-only these days?)
Copied change from old architecture. The stack doesn't load straight away when the sheet opens if animations turned on. Seems plausible, even though thought there were no animations when first load - but the delay doesn't happen with react native modal, don't know why
The root property ensures that modals only dismissed when opening a sheet at the top level scene. Now support stacks in scenes, can open a sheet from a non-top level scene and this mustn't dismiss already opened sheet(s)
Tried so hard to get view controller recycling to work (new fabric feature). But it just doesn't with view controllers. Example number 763 is when opening sheet B from sheet A then flinging sheet B to dismiss - very often this would give a shadow where sheet A should be (just the opaque background but no sheet A). Sometimes, if sheet A was still around, when closing sheet A would get an empty scene behind - so just the black underlay of the stack and no scene. No clue where the problem is but preventing recycling fixes the issues. Was going to try and live with recycling because had it mostly working - maybe only errors sometimes on reload/refresh - but this was the final straw. There's definitely some clash with recycling and uikit - something deep about uiviewcontrollers
Copied over changes from old arch - wrapped the sheet specific (ios 15+) code in available blocks and tested the modal sheet shows up on ios 14
Otherwise doesn't compile
Tried it again and still shows the wrong tab sometimes when opening notifications scene inside a modal non-bottom sheet
Kept bottom sheet for backward compatibility
Now it's used by sheet as well as stack it makes sense to name it what it is
This was referenced Jun 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
Sheetcomponent replacesBottomSheet+ React Native'sModal. For example,The
Sheetsupports childNavigationStackcomponents. Adding predictive gesture back support on Android was tricky because the gesture back goes to the ComponentDialog instead of the FragmentManager by default. The gesture back closed the dialog instead of going back through the stack. Had to build a customFragmentHostCallbackto hook up theFragmentManager'sback stack to theComponentDialog'sOnBackPressedDispatcher.