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: clicking on Pressable located in screen header #2466

Merged
merged 46 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
0044582
remove header offset from RNSScreenStackHeaderConfigShadowNode
coado Oct 29, 2024
fa6ba8c
remove top: '-100%' from ScreenStackHeaderConfig
coado Oct 29, 2024
47212d4
rename topInsetOffset to paddingTop in RNSScreenStackHeaderConfigShad…
coado Oct 29, 2024
7aa8889
Merge branch 'main' into @dmalecki/pressable-in-header
coado Nov 13, 2024
0d80a85
header config frame
coado Nov 13, 2024
8047046
header config state
coado Nov 13, 2024
76681bb
header subview frame size update
coado Nov 13, 2024
92b899a
header subview frame corrections
coado Nov 14, 2024
cfb237e
remove unused import
coado Nov 14, 2024
83fb6f4
flattened header subviews
coado Nov 15, 2024
5d6716d
iOS frame metrics
coado Nov 15, 2024
b3ebaf5
flattening
coado Nov 19, 2024
b9e0124
fix Android
coado Nov 19, 2024
aad021a
fix layout iOS subviews
coado Nov 19, 2024
43ddcb2
header config and subviews position
coado Nov 26, 2024
eca6580
fix not completed layout
coado Nov 26, 2024
0823b03
fix subview hit test bug
coado Nov 26, 2024
61f2f9d
hit testing flattened views on iOS
coado Nov 26, 2024
3b5ee9d
sending subviews frames on fabric
coado Nov 26, 2024
f07b8de
merge
coado Nov 26, 2024
d6e1597
fix Android build paper
coado Nov 28, 2024
723e8a9
FabricEnabledHeaderSubviewGroup empty line
coado Nov 28, 2024
b71e62e
remove RNSScreenStackHeaderConfigShadowNode include
coado Nov 28, 2024
8541afc
remove unused origin from header config state
coado Nov 28, 2024
396abd4
caching header config size, iOS
coado Nov 28, 2024
9799d47
_lastSize recycle
coado Nov 28, 2024
f7bfbd7
remove sending top inset, paper
coado Nov 28, 2024
c71ce45
fix Android build
coado Nov 28, 2024
3337de6
Add reproduction
kkafar Jan 14, 2025
86e64c2
Merge branch 'main' into @dmalecki/pressable-in-header
kkafar Jan 14, 2025
ae32702
Merge branch 'main' into @dmalecki/pressable-in-header
kkafar Jan 14, 2025
b7afeb9
Update TestHeaderTitle & Test2466 examples
kkafar Jan 15, 2025
c29cf9b
Fix regression on header layout with long / short titles
kkafar Jan 15, 2025
d4835ec
Cpp formatter
kkafar Jan 15, 2025
d6a7b4d
Fix updateHeaderConfigState method signature on Paper
kkafar Jan 15, 2025
65b8699
Cleanup example a bit
kkafar Jan 15, 2025
0f8323a
Cleanup Android code a bit
kkafar Jan 15, 2025
ea74898
Make some parts of cpp code Android-only
kkafar Jan 15, 2025
40f550c
Make `applyFrameCorrections` method private
kkafar Jan 15, 2025
8f8eaae
Cosmetic changes in iOS code
kkafar Jan 15, 2025
840c3d9
Remove contentOffset from header config
kkafar Jan 15, 2025
2703a0f
Default initialize contentOffset in SubviewState
kkafar Jan 15, 2025
f6f78c2
A bit more of cleanup & refactor of iOS code
kkafar Jan 15, 2025
45e0cca
Remove contentOffset from header config on Android side
kkafar Jan 15, 2025
f859ccb
Example improvements
kkafar Jan 15, 2025
d62fe03
Handle cases when subview is added after initial render
kkafar Jan 16, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,43 +14,64 @@ abstract class FabricEnabledHeaderConfigViewGroup(
) : ViewGroup(context) {
private var mStateWrapper: StateWrapper? = null

private var lastWidth = 0f
private var lastHeight = 0f
private var lastPaddingStart = 0f
private var lastPaddingEnd = 0f

fun setStateWrapper(wrapper: StateWrapper?) {
mStateWrapper = wrapper
}

fun updatePaddingsFabric(
fun updatePaddings(
paddingStart: Int,
paddingEnd: Int,
) {
updateState(paddingStart, paddingEnd)
// Do nothing on Fabric. This method is used only on Paper.
}

fun updateHeaderConfigState(
width: Int,
height: Int,
paddingStart: Int,
paddingEnd: Int,
) {
updateState(width, height, paddingStart, paddingEnd)
}

@UiThread
fun updateState(
width: Int,
height: Int,
paddingStart: Int,
paddingEnd: Int,
) {
val paddingStartDip: Float = PixelUtil.toDIPFromPixel(paddingStart.toFloat())
val paddingEndDip: Float = PixelUtil.toDIPFromPixel(paddingEnd.toFloat())
val realWidth: Float = PixelUtil.toDIPFromPixel(width.toFloat())
val realHeight: Float = PixelUtil.toDIPFromPixel(height.toFloat())
val realPaddingStart: Float = PixelUtil.toDIPFromPixel(paddingStart.toFloat())
val realPaddingEnd: Float = PixelUtil.toDIPFromPixel(paddingEnd.toFloat())

// Check incoming state values. If they're already the correct value, return early to prevent
// infinite UpdateState/SetState loop.
if (abs(lastPaddingStart - paddingStart) < DELTA &&
abs(lastPaddingEnd - paddingEnd) < DELTA
if (abs(lastWidth - realWidth) < DELTA &&
abs(lastHeight - realHeight) < DELTA &&
abs(lastPaddingStart - realPaddingStart) < DELTA &&
abs(lastPaddingEnd - realPaddingEnd) < DELTA
) {
return
}

lastPaddingStart = paddingStartDip
lastPaddingEnd = paddingEndDip
lastWidth = realWidth
lastHeight = realHeight
lastPaddingStart = realPaddingStart
lastPaddingEnd = realPaddingEnd

val map: WritableMap =
WritableNativeMap().apply {
putDouble("paddingStart", paddingStartDip.toDouble())
putDouble("paddingEnd", paddingEndDip.toDouble())
putDouble("frameWidth", realWidth.toDouble())
putDouble("frameHeight", realHeight.toDouble())
putDouble("paddingStart", realPaddingStart.toDouble())
putDouble("paddingEnd", realPaddingEnd.toDouble())
Comment on lines +73 to +74
Copy link
Member

Choose a reason for hiding this comment

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

Padding must be sent here, well... to account for native padding of support library toolbar. Otherwise we introduce regressions, see the PR that introduced TestHeaderTitle.

}
mStateWrapper?.updateState(map)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package com.swmansion.rnscreens

import android.content.Context
import android.view.ViewGroup
import androidx.annotation.UiThread
import com.facebook.react.bridge.WritableMap
import com.facebook.react.bridge.WritableNativeMap
import com.facebook.react.uimanager.PixelUtil
import com.facebook.react.uimanager.StateWrapper

abstract class FabricEnabledHeaderSubviewGroup(
context: Context?,
) : ViewGroup(context) {
private var mStateWrapper: StateWrapper? = null

fun setStateWrapper(wrapper: StateWrapper?) {
mStateWrapper = wrapper
}

protected fun updateSubviewFrameState(
width: Int,
height: Int,
offsetX: Int,
offsetY: Int,
) {
updateState(width, height, offsetX, offsetY)
}

@UiThread
fun updateState(
width: Int,
height: Int,
offsetX: Int,
offsetY: Int,
) {
val realWidth: Float = PixelUtil.toDIPFromPixel(width.toFloat())
val realHeight: Float = PixelUtil.toDIPFromPixel(height.toFloat())
val offsetXDip: Float = PixelUtil.toDIPFromPixel(offsetX.toFloat())
val offsetYDip: Float = PixelUtil.toDIPFromPixel(offsetY.toFloat())

val map: WritableMap =
WritableNativeMap().apply {
putDouble("frameWidth", realWidth.toDouble())
putDouble("frameHeight", realHeight.toDouble())
putDouble("contentOffsetX", offsetXDip.toDouble())
putDouble("contentOffsetY", offsetYDip.toDouble())
}

mStateWrapper?.updateState(map)
}
}
14 changes: 12 additions & 2 deletions android/src/main/java/com/swmansion/rnscreens/CustomToolbar.kt
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,18 @@ open class CustomToolbar(
) {
super.onLayout(changed, l, t, r, b)

// our children are already laid out
if (!changed) {
return
}

val contentInsetStart = if (navigationIcon != null) contentInsetStartWithNavigation else contentInsetStart
config.updatePaddingsFabric(contentInsetStart, contentInsetEnd)
if (BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
val width = r - l;
val height = b - t;
config.updateHeaderConfigState(width, height, contentInsetStart, contentInsetEnd)
} else {
// our children are already laid out
config.updatePaddings(contentInsetStart, contentInsetEnd)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import com.facebook.react.views.view.ReactViewGroup
@SuppressLint("ViewConstructor")
class ScreenStackHeaderSubview(
context: ReactContext?,
) : ReactViewGroup(context) {
Copy link
Member

Choose a reason for hiding this comment

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

You made FabricEnabledHeaderSubviewGroup inherit directly from ViewGroup, thus removing ReactViewGroup from inheritance chain. Is this on purpose?

) : FabricEnabledHeaderSubviewGroup(context) {
private var reactWidth = 0
private var reactHeight = 0
var type = Type.RIGHT
Expand Down Expand Up @@ -37,11 +37,17 @@ class ScreenStackHeaderSubview(

override fun onLayout(
changed: Boolean,
left: Int,
top: Int,
right: Int,
bottom: Int,
) = Unit
l: Int,
t: Int,
r: Int,
b: Int,
) {
if (changed && BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
val width = r - l;
val height = b - t;
updateSubviewFrameState(width, height, l, t)
}
}

enum class Type {
LEFT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package com.swmansion.rnscreens

import com.facebook.react.bridge.JSApplicationIllegalArgumentException
import com.facebook.react.module.annotations.ReactModule
import com.facebook.react.uimanager.ReactStylesDiffMap
import com.facebook.react.uimanager.StateWrapper
import com.facebook.react.uimanager.ThemedReactContext
import com.facebook.react.uimanager.ViewGroupManager
import com.facebook.react.uimanager.ViewManagerDelegate
Expand Down Expand Up @@ -39,6 +41,17 @@ class ScreenStackHeaderSubviewManager :
}
}

override fun updateState(
view: ScreenStackHeaderSubview,
props: ReactStylesDiffMap?,
stateWrapper: StateWrapper?
): Any? {
if (BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
view.setStateWrapper(stateWrapper)
}
return super.updateState(view, props, stateWrapper)
}

protected override fun getDelegate(): ViewManagerDelegate<ScreenStackHeaderSubview> = delegate

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,15 @@ abstract class FabricEnabledHeaderConfigViewGroup(

fun setStateWrapper(wrapper: StateWrapper?) = Unit

fun updatePaddingsFabric(
// Do nothing on Paper. This method is used only on Fabric.
fun updateHeaderConfigState(
width: Int,
height: Int,
paddingStart: Int,
paddingEnd: Int,
) = Unit

fun updatePaddings(
paddingStart: Int,
paddingEnd: Int,
) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.swmansion.rnscreens

import android.content.Context
import android.view.ViewGroup
import com.facebook.react.uimanager.StateWrapper

abstract class FabricEnabledHeaderSubviewGroup(context: Context?): ViewGroup(context) {

fun setStateWrapper(wrapper: StateWrapper?) = Unit

// Fabric only
protected fun updateSubviewFrameState(
width: Int,
height: Int,
offsetX: Int,
offsetY: Int
) = Unit
}
155 changes: 155 additions & 0 deletions apps/src/tests/Test2466.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
import { Header } from '@react-navigation/elements';
import { NavigationContainer } from '@react-navigation/native';
import { createNativeStackNavigator, NativeStackNavigationProp } from '@react-navigation/native-stack';
import React, { ForwardedRef } from 'react';
import { findNodeHandle, GestureResponderEvent, Pressable, PressableProps, StyleSheet, Text, View } from 'react-native';

type StackParamList = {
Home: undefined,
}

type RouteProps = {
navigation: NativeStackNavigationProp<StackParamList>;
}

type PressableState = 'pressed-in' | 'pressed' | 'pressed-out'


const Stack = createNativeStackNavigator<StackParamList>();

const PressableWithFeedback = React.forwardRef((props: PressableProps, ref: ForwardedRef<View>): React.JSX.Element => {
const [pressedState, setPressedState] = React.useState<PressableState>('pressed-out');

const onPressInCallback = React.useCallback((e: GestureResponderEvent) => {
console.log('Pressable onPressIn', {
locationX: e.nativeEvent.locationX,
locationY: e.nativeEvent.locationY,
pageX: e.nativeEvent.pageX,
pageY: e.nativeEvent.pageY,
});
setPressedState('pressed-in');
props.onPressIn?.(e);
}, [props]);

const onPressCallback = React.useCallback((e: GestureResponderEvent) => {
console.log('Pressable onPress');
setPressedState('pressed');
props.onPress?.(e);
}, [props]);

const onPressOutCallback = React.useCallback((e: GestureResponderEvent) => {
console.log('Pressable onPressOut');
setPressedState('pressed-out');
props.onPressOut?.(e);
}, [props]);

const onResponderMoveCallback = React.useCallback((e: GestureResponderEvent) => {
console.log('Pressable onResponderMove');
props.onResponderMove?.(e);
}, [props]);

const contentsStyle = pressedState === 'pressed-out'
? styles.pressablePressedOut
: (pressedState === 'pressed'
? styles.pressablePressed
: styles.pressablePressedIn);

return (
<View ref={ref} style={[contentsStyle]}>
<Pressable
onPressIn={onPressInCallback}
onPress={onPressCallback}
onPressOut={onPressOutCallback}
onResponderMove={onResponderMoveCallback}
>
{props.children}
</Pressable>
</View>

);
});

function HeaderTitle(): React.JSX.Element {
return (
<PressableWithFeedback
onLayout={event => {
const { x, y, width, height } = event.nativeEvent.layout;
console.log('Title onLayout', { x, y, width, height });
}}
onPressIn={() => {
console.log('Pressable onPressIn');
}}
onPress={() => console.log('Pressable onPress')}
onPressOut={() => console.log('Pressable onPressOut')}
onResponderMove={() => console.log('Pressable onResponderMove')}
ref={node => {
console.log(findNodeHandle(node));
node?.measure((x, y, width, height, pageX, pageY) => {
console.log('header component measure', { x, y, width, height, pageX, pageY });
});
}}
>
<View style={{ height: 40, justifyContent: 'center', alignItems: 'center' }}>
<Text style={{ alignItems: 'center' }}>Regular Pressable</Text>
</View>
</PressableWithFeedback>
);
}

function HeaderLeft(): React.JSX.Element {
return (
<HeaderTitle />
);
}

function Home(_: RouteProps): React.JSX.Element {
return (
<View style={{ flex: 1, backgroundColor: 'rgba(0, 0, 0, .8)' }}
>
<View style={{ flex: 1, alignItems: 'center', marginTop: 48 }}>
<PressableWithFeedback
onPressIn={() => console.log('Pressable onPressIn')}
onPress={() => console.log('Pressable onPress')}
onPressOut={() => console.log('Pressable onPressOut')}
>
<View style={{ height: 40, width: 200, justifyContent: 'center', alignItems: 'center' }}>
<Text style={{ alignItems: 'center' }}>Regular Pressable</Text>
</View>
</PressableWithFeedback>
</View>
</View>
);
}

function App(): React.JSX.Element {
return (
<NavigationContainer>
<Stack.Navigator>
<Stack.Screen
name="Home"
component={Home}
options={{
headerTitle: HeaderTitle,
headerLeft: HeaderLeft,
headerRight: HeaderLeft,
}}
/>
</Stack.Navigator>
</NavigationContainer>
);
}

const styles = StyleSheet.create({
pressablePressedIn: {
backgroundColor: 'lightsalmon',
},
pressablePressed: {
backgroundColor: 'crimson',
},
pressablePressedOut: {
backgroundColor: 'lightseagreen',
},
});


export default App;
Loading
Loading