Skip to content

Conversation

Copy link

Copilot AI commented Oct 24, 2025

Summary

Implements dev menu support for Fabric (the new architecture's renderer) on macOS, enabling the right-click context menu to show the dev menu in Fabric-based apps.

Problem

In Paper (the legacy renderer), the dev menu was enabled on right-click via an override in RCTRootView:

- (NSMenu *)menuForEvent:(NSEvent *)event
{
  NSMenu *menu = nil;
#if __has_include("RCTDevMenu.h") && RCT_DEV
  menu = [[_bridge devMenu] menu];
#endif
  // ...
  return menu;
}

However, Fabric (the new architecture) had no equivalent implementation in its view hierarchy, leaving users without dev menu access when using Fabric.

Solution

This PR adds dev menu support to Fabric with a clean architectural separation:

1. Bridge-independent view layer (RCTSurfaceHostingView)

Implemented menuForEvent: in RCTSurfaceHostingView (Fabric's equivalent of RCTRootView) that accesses DevMenu from the contextContainer:

- (NSMenu *)menuForEvent:(NSEvent *)event
{
  NSMenu *menu = nil;
#if __has_include(<React/RCTDevMenu.h>) && RCT_DEV
  // Try to get DevMenu from the contextContainer via the surface presenter
  // This works in Fabric architecture where DevMenu is stored in contextContainer
  if ([self.surface respondsToSelector:@selector(surfacePresenter)]) {
    RCTSurfacePresenter *surfacePresenter = [self.surface performSelector:@selector(surfacePresenter)];
    if (surfacePresenter) {
      auto contextContainer = surfacePresenter.contextContainer;
      if (contextContainer) {
        auto optionalDevMenu = contextContainer->find<std::shared_ptr<void>>("RCTDevMenu");
        if (optionalDevMenu) {
          RCTDevMenu *devMenu = facebook::react::unwrapManagedObject(optionalDevMenu.value());
          if (devMenu) {
            menu = [devMenu menu];
          }
        }
      }
    }
  }
  
  // Fall back to notification-based approach if contextContainer access fails
  if (menu == nil) {
    [[NSNotificationCenter defaultCenter] postNotificationName:RCTShowDevMenuNotification object:nil];
    return nil;
  }
#endif
  
  if (menu == nil) {
    menu = [super menuForEvent:event];
  }
  return menu;
}

2. DevMenu in contextContainer (RCTSurfacePresenterBridgeAdapter)

Added DevMenu to the contextContainer in the bridge adapter layer, making it available to Fabric components:

#if __has_include(<React/RCTDevMenu.h>) && RCT_DEV
  RCTDevMenu *devMenu = [bridge devMenu];
  if (devMenu) {
    contextContainer->insert("RCTDevMenu", wrapManagedObject(devMenu));
  }
#endif

Implementation Details

  • Bridge-independent view layer: RCTSurfaceHostingView accesses DevMenu through the contextContainer, not directly via bridge
  • Returns actual NSMenu: Like Paper, returns the menu object synchronously for proper right-click behavior
  • Clean separation: Bridge dependencies are isolated in the adapter layer, not in the view layer
  • ContextContainer integration: DevMenu is stored in and accessed from Fabric's contextContainer
  • Notification fallback: Falls back to notification-based approach if contextContainer access is unavailable
  • Conditional compilation: Only included in dev builds via RCT_DEV guards

Testing

Manual testing required:

  1. Run a Fabric-enabled macOS app in dev mode
  2. Right-click on the React Native view
  3. Verify the dev menu appears with options like "Reload", "Toggle Element Inspector", etc.

Fixes #2734

Original prompt

This section details on the original issue you should resolve

<issue_title>[Fabric] Dev Menu is missing</issue_title>
<issue_description>In Paper (the legacy renderer), we enabled the dev menu on right click with the following override in RCTRootView: https://github.com/microsoft/react-native-macos/blob/721a5aa852add0a8dc0beb09b86c47d369268199/packages/react-native/React/Base/RCTRootView.m#L493C1-L508C17

We don't have any equivalent in Fabric (the new architecture's renderer). We will need to add some similar override in something like RCTSurfaceHostingView or RCTSurfaceRootView . We will need to be careful to not use the bridge to grab the RCTDevMenu module, as there is no bridge in the new architecture.
</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Add dev menu support in Fabric renderer [macOS] Add dev menu support for Fabric (new architecture) Oct 24, 2025
Copilot AI requested a review from Saadnajmi October 24, 2025 20:20
Copilot finished work on behalf of Saadnajmi October 24, 2025 20:20
@Saadnajmi
Copy link
Collaborator

@copilot can we remove any fallbacks to the bridge in this PR?

@Saadnajmi
Copy link
Collaborator

@copilot can we remove any fallbacks to the bridge in this PR?

Actually, keep them in the bridge adapter, but not SurfaceHostingView

Copy link
Author

Copilot AI commented Oct 24, 2025

@copilot can we remove any fallbacks to the bridge in this PR?

Done! Removed all bridge access. The implementation now uses only the notification-based approach (RCTShowDevMenuNotification) which is fully bridge-independent. Changes in commit 28fb425.

Copilot finished work on behalf of Saadnajmi October 24, 2025 20:29
Copy link
Author

Copilot AI commented Oct 24, 2025

@copilot can we remove any fallbacks to the bridge in this PR?

Actually, keep them in the bridge adapter, but not SurfaceHostingView

Done! Restored DevMenu in the bridge adapter's contextContainer while keeping SurfaceHostingView bridge-independent (notification-only). Changes in commit ee90ce2.

Copilot finished work on behalf of Saadnajmi October 24, 2025 20:33
@Saadnajmi
Copy link
Collaborator

@copilot Can we not get the RCTDevMenu module in RCTSurfaceHostingView from the contect container? I don't think the notification approach will work well

Copy link
Author

Copilot AI commented Oct 24, 2025

@copilot Can we not get the RCTDevMenu module in RCTSurfaceHostingView from the contect container? I don't think the notification approach will work well

Done! Updated to access RCTDevMenu from the contextContainer via the surface presenter. The implementation now returns the actual NSMenu (like Paper does) instead of using notifications, with a fallback to notifications if contextContainer access fails. Changes in commit 6506433.

Copilot finished work on behalf of Saadnajmi October 24, 2025 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Fabric] Dev Menu is missing

2 participants