Support drawer menus opening as overlay instead of pushing content in ios #7986

liatnetach · 2025-03-30T13:39:12Z

This PR adds the ability to configure how side drawer menus open, introducing an overlay mode in addition to the traditional push-content behavior.

… ios (#7984) * add example in playground and some animations changes with Yogi * fix drag animations (open+close) in left drawer and add overlay on center * support all features in right drawer * supoprt both old and new opening mode using openAboveScreen param * connect the new parameter to rnn platform * update param from boolean to enum * clean * install * update lock * fix unit

Copilot

Pull Request Overview

This PR introduces overlay mode for side drawer menus on iOS, allowing the drawer to open above the content rather than pushing it.

Added new test IDs for left and right menus.
Added new functions and UI buttons in SetRootScreen to configure left and right menus with overlay behavior.
Updated the Options interface to support the new openMode property.

Reviewed Changes

Copilot reviewed 5 out of 10 changed files in this pull request and generated no comments.

File	Description
playground/src/testIDs.ts	Added new test IDs for the left and right side menus.
playground/src/screens/SetRootScreen.tsx	Added buttons and functions to configure side menus in overlay mode for iOS.
lib/src/interfaces/Options.ts	Extended the Options interface with an openMode property for side menus.

Files not reviewed (5)

lib/ios/RNNSideMenu/MMDrawerController/MMDrawerController.h: Language not supported
lib/ios/RNNSideMenuPresenter.m: Language not supported
lib/ios/RNNSideMenuSideOptions.h: Language not supported
lib/ios/RNNSideMenuSideOptions.m: Language not supported
playground/ios/NavigationTests/RNNSideMenuPresenterTest.m: Language not supported

Comments suppressed due to low confidence (2)

playground/src/screens/SetRootScreen.tsx:336

[nitpick] The component id 'sideMenu' is used for the left menu configuration; consider renaming it to 'leftSideMenu' for clarity and to avoid confusion with the right menu configuration.

id: 'sideMenu',

playground/src/screens/SetRootScreen.tsx:376

[nitpick] The component id 'sideMenu' is used for the right menu configuration; consider renaming it to 'rightSideMenu' for clearer distinction between the left and right menus.

id: 'sideMenu',

d4vidi

Impressive overall, but needs to be seriously improved before we can merge this in. Please see notes 🙏🏻

d4vidi · 2025-04-06T11:28:36Z

playground/src/screens/SetRootScreen.tsx

+        <Button
+          label="Set Root with left menu"
+          testID={SET_ROOT_WITH_LEFT_MENU}
+          onPress={this.setRootWithLeftMenu}
+        />
+        <Button
+          label="Set Root with right menu"
+          testID={SET_ROOT_WITH_RIGHT_MENU}
+          onPress={this.setRootWithRightMenu}
+        />


I think better place for this would be the side-menu modal (Layouts > SideMenu btn) --

...

Did you resort to set-root on purpose, in order to simplify?

If so, why split the left-menu and right-menu into 2 different sub-screens, rather than having 1 screen with both? It's work quirky this way, with the "right menu" button breaking the app when set with the left-menu only

it's in root and not modal cause it shows our actual use case, we set the drawer in the root hierarchy - modal is different case

np, changing to one button with both menus :)

For RNNav there's no difference. Please align this with the existing side-menus test. It needs to pass all existing tests there - including device rotation and drawer width changes; Please go as far as putting this under a test.each for both open modes (new and old).
1.1. We must add a screenshot test for both cases
1.2 If you're really concerned about this working as a root, we can add just 1 simple, happy-flow test covering exactly that

@d4vidi Where can I find the existing screenshot tests? can you please add a reference?
or maybe did you meant the side menu e2e file?

playground/src/screens/SetRootScreen.tsx

lib/src/interfaces/Options.ts

d4vidi · 2025-04-06T13:00:14Z

lib/src/interfaces/Options.ts

+   * @default 'pushContent'
+
+   */
+  openMode?: 'pushContent' | 'aboveContent';


openMode - to me, sounds like a configuration of which animation to use when opening the drawer.

Suggestion: Rename openMode → layoutType

BTW is shouldStretchDrawer applicable with aboveContent? If not, need to specify that too in the description

what shouldStretchDrawer is for? it should be default true but I can't see any indication to something like stretching content..

regarding the renaming layoutType sounds to general to me, I will prefer to keep the openMode naming 🙏

Stretchy Drawer By default, the side drawer will stretch if the user pans past the maximum drawer width. This gives a playful stretch effect. You can disable this by setting shouldStretchDrawer to NO, or you can make your own overshoot animation by creating a custom visual state block and setting up custom transforms for when percentVisible is greater than 1.0

https://github.com/mutualmobile/MMDrawerController

thanks, Ill document that it is not supported with above content mode :)

lib/ios/RNNSideMenuSideOptions.m

d4vidi · 2025-04-06T13:42:20Z

lib/ios/RNNSideMenu/MMDrawerController/MMDrawerController.m

+                NSTimeInterval duration = MAX(distance / ABS(velocity), MMDrawerMinimumAnimationDuration);
+
+                // Animate opening
+                [UIView animateWithDuration:(animated ? duration : 0.0)


This call needs to be extracted onto a function and reused in all modes (overlay/push)

in each call the settings are different we can't share this code, it's not really exactly the same 🤔 maybe I'm missing something?

@d4vidi can be resolved?

d4vidi · 2025-04-06T13:43:08Z

lib/ios/RNNSideMenu/MMDrawerController/MMDrawerController.m

+                             self.rightDrawerOpenMode;
+
+            if (openMode == MMDrawerOpenModeAboveContent) {
+                // OVERLAY MODE


2 dedicated sub-functions: One for each mode (overlay, push-content)

@d4vidi can be resolved?

lib/ios/RNNSideMenu/MMDrawerController/MMDrawerController.m

d4vidi · 2025-04-06T13:54:36Z

lib/ios/RNNSideMenu/MMDrawerController/MMDrawerController.m

+                             self.leftDrawerOpenMode : 
+                             self.rightDrawerOpenMode;
+
+        if (openMode == MMDrawerOpenModeAboveContent) {


Really nice, but why is this custom handling needed just for the overlay mode (but not push mode)?

Anyways it would be impossible to merge this code as-is. It needs to be extracted onto a separate module, to handle all cases - UIGestureRecognizerStateBegan, UIGestureRecognizerStateChanged and UIGestureRecognizerStateEnded. Preferably, it should be delegated to the original mode's code which I assume already has this (?)....

@d4vidi I don't understand the second comment, can you please clarify? 🙏

regarding the first question the whole calculation is different between the modes cause in the original mode we calculate the center view position and in the new mode we needs the drawer itself
the actual element who moves is different
thats why there are many differences between the two regarding the animation calculations

I see, thanks. Well we need to improve the code here. The function (panGestureCallback) is already massive, let's try to extract what we can onto sub-functions, at least. See 9c9834b#diff-a32c08640d39c005d4c2d2ad17f75707ab7286e61ed5b286fbf4dbb04541ba07R1409 as an example.

Ive split panGestureCallback method into small methods, you can see this commit
🙂

liatnetach · 2025-04-23T09:05:57Z

#rebuild

…open-mode

lib/ios/RNNSideMenuSideOptions.m

lib/ios/RNNSideMenu/MMDrawerController/MMDrawerController.m

d4vidi · 2025-05-13T10:26:07Z

lib/ios/RNNSideMenu/MMDrawerController/MMDrawerController.m

-                [self sideDrawerViewControllerForSide:self.openSide];
-            [sideDrawerViewController beginAppearanceTransition:NO animated:NO];
-            [sideDrawerViewController endAppearanceTransition];
+        if (openMode == MMDrawerOpenModeAboveContent) {


... Same here. The change here is massive, you need to find a way to elegantly extract this massive amount of logic onto several sub-methods. Also, please minimize the extra-comments = fluff 😄

Ive split panGestureCallback method into small methods, you can see this commit
🙂

d4vidi

Hey @liatnetach, some further discussion - above. Still a lot to be done but please keep up the good work. It's mainly about:

Getting the side-menu thoroughly tested in the SideMenu's suite (not the SetRoot one) - "full matrix" (all tests for both modes)
Massively cleaning up and reorg'ing the work in panGestureCallback
Fix an odd bug - the drawer disappears unexpectedly when panning out

Simulator.Screen.Recording.-.iPhone.13.Pro.Max.-.2025-05-13.at.13.30.11.mp4

liatnetach · 2025-05-18T15:13:14Z

Hey @d4vidi 🙂
I've answered and pushed everything you asked for (including the bug fix - thanks for noticing!).
Please resolve what can be resolved, and let me know if there's anything else. 🙂

liatnetach requested a review from d4vidi March 30, 2025 13:39

liatnetach assigned d4vidi Mar 30, 2025

d4vidi added platform: iOS type: accepted/enhancement feature: SideMenu labels Mar 30, 2025

d4vidi force-pushed the support-drawer-menu-open-mode branch from 0a1937a to f1d3449 Compare March 31, 2025 12:08

d4vidi mentioned this pull request Mar 31, 2025

Revert #7984 (sidemenu as overlay) temporarily #7989

Merged

d4vidi requested a review from Copilot April 3, 2025 16:24

Copilot AI reviewed Apr 3, 2025

View reviewed changes

d4vidi requested changes Apr 6, 2025

View reviewed changes

liatnetach added 2 commits April 15, 2025 16:00

clean code using sub functions anf few more pr suggestions

29131c7

Merge branch '7.x.x' into support-drawer-menu-open-mode

4803f82

liatnetach force-pushed the support-drawer-menu-open-mode branch from f1d3449 to 4803f82 Compare April 15, 2025 13:03

add e2e

3183838

liatnetach requested a review from d4vidi April 15, 2025 13:28

update test

d98e6cd

liatnetach and others added 2 commits April 23, 2025 13:32

add e2e

8a591c3

Merge remote-tracking branch 'origin/7.x.x' into support-drawer-menu-…

7e7e97b

…open-mode

d4vidi reviewed May 12, 2025

View reviewed changes

lib/ios/RNNSideMenuSideOptions.m Show resolved Hide resolved

d4vidi reviewed May 12, 2025

View reviewed changes

lib/ios/RNNSideMenu/MMDrawerController/MMDrawerController.m Outdated Show resolved Hide resolved

d4vidi added 2 commits May 13, 2025 12:35

Code clean-up

588bd1d

Improve code for drawer pan start

9c9834b

d4vidi reviewed May 13, 2025

View reviewed changes

d4vidi requested changes May 13, 2025

View reviewed changes

liatnetach added 4 commits May 15, 2025 11:32

split panGestureCallback

640ca2e

add same tests for above screen

5390499

update tests

61a939b

fix tests

dd7ddc4

split ios android close case + fix anumation bug

465146c

liatnetach force-pushed the support-drawer-menu-open-mode branch from 381fe1a to 465146c Compare May 15, 2025 17:36

liatnetach added 4 commits May 15, 2025 21:34

revert fix

d73c98b

update the percentVisible condition

7ba9697

fix shouldStretch for original mode

cc29eb0

update description

acea9ee

liatnetach requested a review from d4vidi May 18, 2025 14:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support drawer menus opening as overlay instead of pushing content in ios #7986

Support drawer menus opening as overlay instead of pushing content in ios #7986

liatnetach commented Mar 30, 2025

Copilot AI left a comment

d4vidi left a comment

d4vidi Apr 6, 2025

liatnetach Apr 15, 2025

d4vidi May 11, 2025 •

edited

Loading

liatnetach May 15, 2025 •

edited

Loading

d4vidi Apr 6, 2025

d4vidi Apr 6, 2025

liatnetach Apr 15, 2025

liatnetach Apr 15, 2025

d4vidi May 12, 2025 •

edited

Loading

liatnetach May 18, 2025

d4vidi Apr 6, 2025

liatnetach Apr 15, 2025

liatnetach May 15, 2025

d4vidi Apr 6, 2025

liatnetach May 15, 2025

d4vidi Apr 6, 2025

d4vidi Apr 6, 2025

liatnetach Apr 15, 2025

d4vidi May 13, 2025

liatnetach May 15, 2025

liatnetach commented Apr 23, 2025

d4vidi May 13, 2025

liatnetach May 15, 2025

d4vidi left a comment

liatnetach commented May 18, 2025

Support drawer menus opening as overlay instead of pushing content in ios #7986

Are you sure you want to change the base?

Support drawer menus opening as overlay instead of pushing content in ios #7986

Conversation

liatnetach commented Mar 30, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

d4vidi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d4vidi May 11, 2025 • edited Loading

Choose a reason for hiding this comment

liatnetach May 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d4vidi May 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liatnetach commented Apr 23, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d4vidi left a comment

Choose a reason for hiding this comment

liatnetach commented May 18, 2025

d4vidi May 11, 2025 •

edited

Loading

liatnetach May 15, 2025 •

edited

Loading

d4vidi May 12, 2025 •

edited

Loading