Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix inspector reporting wrong frames when using DrawerLayoutAndroid on the new arch #44426

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import android.view.MotionEvent;
import android.view.View;
import android.view.accessibility.AccessibilityEvent;

import androidx.annotation.NonNull;
import androidx.core.view.AccessibilityDelegateCompat;
import androidx.core.view.ViewCompat;
import androidx.core.view.accessibility.AccessibilityNodeInfoCompat;
Expand All @@ -19,8 +21,12 @@
import com.facebook.infer.annotation.Nullsafe;
import com.facebook.react.R;
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.bridge.ReadableMap;
import com.facebook.react.bridge.WritableNativeMap;
import com.facebook.react.common.ReactConstants;
import com.facebook.react.uimanager.PixelUtil;
import com.facebook.react.uimanager.ReactAccessibilityDelegate.AccessibilityRole;
import com.facebook.react.uimanager.StateWrapper;
import com.facebook.react.uimanager.events.NativeGestureUtil;

/**
Expand All @@ -34,6 +40,8 @@ class ReactDrawerLayout extends DrawerLayout {
private int mDrawerPosition = Gravity.START;
private int mDrawerWidth = DEFAULT_DRAWER_WIDTH;
private boolean mDragging = false;
private boolean mOpened = false;
private StateWrapper mStateWrapper = null;

public ReactDrawerLayout(ReactContext reactContext) {
super(reactContext);
Expand Down Expand Up @@ -61,6 +69,26 @@ public void onInitializeAccessibilityEvent(View host, AccessibilityEvent event)
}
}
});

this.addDrawerListener(new DrawerListener() {
@Override
public void onDrawerSlide(@NonNull View drawerView, float slideOffset) {}

@Override
public void onDrawerOpened(@NonNull View drawerView) {
mOpened = true;
updateState();
}

@Override
public void onDrawerClosed(@NonNull View drawerView) {
mOpened = false;
updateState();
}

@Override
public void onDrawerStateChanged(int newState) {}
});
}

@Override
Expand Down Expand Up @@ -109,6 +137,63 @@ public boolean onTouchEvent(MotionEvent ev) {
setDrawerProperties();
}

@Override
protected void onLayout(boolean changed, int l, int t, int r, int b) {
super.onLayout(changed, l, t, r, b);
updateState();
}

public void setStateWrapper(StateWrapper stateWrapper) {
this.mStateWrapper = stateWrapper;
}

public void updateState() {
this.updateState(mOpened, mDrawerPosition == Gravity.START, PixelUtil.toDIPFromPixel(getWidth()), PixelUtil.toDIPFromPixel(mDrawerWidth));
}
j-piasecki marked this conversation as resolved.
Show resolved Hide resolved

private void updateState(boolean isOpen, boolean onLeft, float containerWidth, float drawerWidth) {
if (this.mStateWrapper == null) {
return;
}

ReadableMap currentState = mStateWrapper.getStateData();

if (currentState == null) {
return;
}

double delta = 0.9f;
boolean stateOnLeft = true;
boolean stateDrawerOpened = false;
double stateContainerWidth = 0;
double stateDrawerWidth = 0;

if (currentState.hasKey("drawerOnLeft")) {
stateOnLeft = currentState.getBoolean("drawerOnLeft");
}

if (currentState.hasKey("drawerOpened")) {
stateDrawerOpened = currentState.getBoolean("drawerOpened");
}

if (currentState.hasKey("containerWidth")) {
stateContainerWidth = currentState.getDouble("containerWidth");
}

if (currentState.hasKey("drawerWidth")) {
stateDrawerWidth = currentState.getDouble("drawerWidth");
}

if (isOpen != stateDrawerOpened || onLeft != stateOnLeft || Math.abs(stateContainerWidth - containerWidth) > delta || Math.abs(stateDrawerWidth - drawerWidth) > delta) {
WritableNativeMap newState = new WritableNativeMap();
newState.putBoolean("drawerOnLeft", onLeft);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about assigning these strings to private final static fields to eliminate risk of typos?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what's the convention, in other places plain strings are used as well:

newStateData.putDouble("screenWidth", realWidth.toDouble())
newStateData.putDouble("screenHeight", realHeight.toDouble())

newStateData.putInt("mostRecentEventCount", mEditText.incrementAndGetEventCounter());
newStateData.putInt("opaqueCacheId", mEditText.getId());

newState.putBoolean("drawerOpened", isOpen);
newState.putDouble("drawerWidth", drawerWidth);
newState.putDouble("containerWidth", containerWidth);
mStateWrapper.updateState(newState);
}
}

// Sets the properties of the drawer, after the navigationView has been set.
/* package */ void setDrawerProperties() {
if (this.getChildCount() == 2) {
Expand All @@ -118,6 +203,7 @@ public boolean onTouchEvent(MotionEvent ev) {
layoutParams.width = mDrawerWidth;
drawerView.setLayoutParams(layoutParams);
drawerView.setClickable(true);
updateState();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import com.facebook.react.common.ReactConstants;
import com.facebook.react.module.annotations.ReactModule;
import com.facebook.react.uimanager.PixelUtil;
import com.facebook.react.uimanager.ReactStylesDiffMap;
import com.facebook.react.uimanager.StateWrapper;
import com.facebook.react.uimanager.ThemedReactContext;
import com.facebook.react.uimanager.UIManagerHelper;
import com.facebook.react.uimanager.ViewGroupManager;
Expand Down Expand Up @@ -185,6 +187,14 @@ public boolean needsCustomLayoutForChildren() {
return true;
}

@Nullable
@Override
public Object updateState(@NonNull ReactDrawerLayout view, ReactStylesDiffMap props, StateWrapper stateWrapper) {
view.setStateWrapper(stateWrapper);
view.updateState();
return null;
}

@Override
public @Nullable Map<String, Integer> getCommandsMap() {
return MapBuilder.of("openDrawer", OPEN_DRAWER, "closeDrawer", CLOSE_DRAWER);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ add_react_common_subdir(react/renderer/components/view)
add_react_common_subdir(react/renderer/components/switch)
add_react_common_subdir(react/renderer/components/textinput)
add_react_common_subdir(react/renderer/components/progressbar)
add_react_common_subdir(react/renderer/components/drawerlayout)
add_react_common_subdir(react/renderer/components/root)
add_react_common_subdir(react/renderer/components/image)
add_react_common_subdir(react/renderer/components/legacyviewmanagerinterop)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ target_link_libraries(
rrc_image
rrc_modal
rrc_progressbar
rrc_drawerlayout
rrc_root
rrc_scrollview
rrc_switch
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <react/renderer/components/image/ImageComponentDescriptor.h>
#include <react/renderer/components/modal/ModalHostViewComponentDescriptor.h>
#include <react/renderer/components/progressbar/AndroidProgressBarComponentDescriptor.h>
#include <react/renderer/components/drawerlayout/AndroidDrawerLayoutComponentDescriptor.h>
#include <react/renderer/components/rncore/ComponentDescriptors.h>
#include <react/renderer/components/scrollview/ScrollViewComponentDescriptor.h>
#include <react/renderer/components/text/ParagraphComponentDescriptor.h>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

cmake_minimum_required(VERSION 3.13)
set(CMAKE_VERBOSE_MAKEFILE on)

add_compile_options(
-fexceptions
-frtti
-std=c++20
-Wall
-Wpedantic
-DLOG_TAG=\"Fabric\")

file(GLOB rrc_drawerlayout_SRC CONFIGURE_DEPENDS android/react/renderer/components/drawerlayout/*.cpp)
add_library(rrc_drawerlayout STATIC ${rrc_drawerlayout_SRC})

target_include_directories(rrc_drawerlayout
PUBLIC
${CMAKE_CURRENT_SOURCE_DIR}/android/
)

target_link_libraries(rrc_drawerlayout
glog
fbjni
folly_runtime
glog_init
react_codegen_rncore
react_debug
react_render_componentregistry
react_render_core
react_render_debug
react_render_graphics
react_render_uimanager
reactnativejni
rrc_view
yoga
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

#pragma once

#include <react/renderer/components/drawerlayout/AndroidDrawerLayoutShadowNode.h>
#include "AndroidDrawerLayoutShadowNode.h"

namespace facebook::react {

/*
* Descriptor for <AndroidDrawerLayout> component.
*/
class AndroidDrawerLayoutComponentDescriptor final
: public ConcreteComponentDescriptor<AndroidDrawerLayoutShadowNode> {
public:
AndroidDrawerLayoutComponentDescriptor(
const ComponentDescriptorParameters& parameters)
: ConcreteComponentDescriptor(parameters) {}
};

} // namespace facebook::react
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

#include "AndroidDrawerLayoutShadowNode.h"

#include <react/renderer/components/drawerlayout/AndroidDrawerLayoutShadowNode.h>
#include <react/renderer/core/LayoutContext.h>

namespace facebook::react {

extern const char AndroidDrawerLayoutComponentName[] = "AndroidDrawerLayout";

Point AndroidDrawerLayoutShadowNode::getContentOriginOffset() const {
const auto& stateData = getStateData();

if (stateData.drawerOpened && stateData.drawerOnLeft) {
return {0, 0};
} else if (stateData.drawerOpened && !stateData.drawerOnLeft) {
return {stateData.containerWidth - stateData.drawerWidth, 0};
}

return {stateData.drawerOnLeft ? -stateData.containerWidth : stateData.containerWidth, 0};
}

} // namespace facebook::react
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

#pragma once

#include <react/renderer/components/drawerlayout/AndroidDrawerLayoutState.h>
#include <react/renderer/components/rncore/EventEmitters.h>
#include <react/renderer/components/rncore/Props.h>
#include <react/renderer/components/view/ConcreteViewShadowNode.h>

namespace facebook::react {

extern const char AndroidDrawerLayoutComponentName[];

/*
* `ShadowNode` for <AndroidDrawerLayout> component.
*/
class AndroidDrawerLayoutShadowNode final : public ConcreteViewShadowNode<
AndroidDrawerLayoutComponentName,
AndroidDrawerLayoutProps,
AndroidDrawerLayoutEventEmitter,
AndroidDrawerLayoutState> {
public:
using ConcreteViewShadowNode::ConcreteViewShadowNode;

#pragma mark - LayoutableShadowNode

Point getContentOriginOffset() const override;

};

} // namespace facebook::react
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

#include "AndroidDrawerLayoutState.h"

namespace facebook::react {

#ifdef ANDROID
folly::dynamic AndroidDrawerLayoutState::getDynamic() const {
return folly::dynamic::object("drawerOpened", drawerOpened)("drawerOnLeft", drawerOnLeft)(
"drawerWidth", drawerWidth)("containerWidth", containerWidth);
}
#endif

} // namespace facebook::react
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

#pragma once

#include <react/renderer/core/graphicsConversions.h>
#include <react/renderer/graphics/Float.h>

#include <folly/dynamic.h>
#include <react/renderer/mapbuffer/MapBuffer.h>
#include <react/renderer/mapbuffer/MapBufferBuilder.h>

namespace facebook::react {

/*
* State for <AndroidDrawerLayout> component.
*/
class AndroidDrawerLayoutState final {
public:
using Shared = std::shared_ptr<const AndroidDrawerLayoutState>;

AndroidDrawerLayoutState(){};
AndroidDrawerLayoutState(
const AndroidDrawerLayoutState& previousState,
folly::dynamic data)
: drawerWidth((Float)data["drawerWidth"].getDouble()),
containerWidth((Float)data["containerWidth"].getDouble()),
drawerOnLeft(data["drawerOnLeft"].getBool()),
drawerOpened(data["drawerOpened"].getBool()) {};

const Float drawerWidth = 0;
const Float containerWidth = 0;
const bool drawerOnLeft = true;
const bool drawerOpened = false;

folly::dynamic getDynamic() const;
MapBuffer getMapBuffer() const {
return MapBufferBuilder::EMPTY();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really safe to remove an empty map buffer here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is, as long as we're not calling stateWrapper.getStateDataMapBuffer(), which we're not doing, instead calling getStateData which returns the dynamic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's done exactly the same way in modal component:

MapBuffer getMapBuffer() const {
return MapBufferBuilder::EMPTY();
};

};
};

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,6 @@ export const Commands: NativeCommands = codegenNativeCommands<NativeCommands>({
supportedCommands: ['openDrawer', 'closeDrawer'],
});

export default (codegenNativeComponent<NativeProps>(
'AndroidDrawerLayout',
): NativeType);
export default (codegenNativeComponent<NativeProps>('AndroidDrawerLayout', {
interfaceOnly: true,
}): NativeType);