Skip to content

Commit

Permalink
Merge Android ViewNativeComponent ViewConfig into BaseViewConfig (#47105
Browse files Browse the repository at this point in the history
)

Summary:
Pull Request resolved: #47105

`codegenNativeComponent` expects we declare props as extending `ViewProps`, but the generated ViewConfig extends from BaseViewConfig.

This doesn't matter on iOS, where ViewProps are implemented more uniformly across components, but on Android, means we miss about 40 view props, since `ReactViewManager` backing `<View>` supports quite a bit more than `BaseViewManager`. This means that any components which extend `ReactViewManager` have some ViewProps omitted.

In this diff, I went with the solution of moving the props specific to View's ViewConfig to BaseViewConfig. This means the SVC may treat more props as valid than the underlying native component, but this should be safe compared to undercounting, and this will make future moves from ReactViewManager to BaseViewManager safer.

BaseViewConfig was already exposing props not supported by BaseViewManager, like those related to border widths (which effect LayoutShadowNode, but cannot be drawn out of the box?), so this shouldn't be too out there.

The alternative to this was to publicly expose the View ViewConfig and extend from that in codegen instead, but this seemed more complicated without much benefit.

Changelog:
[Android][Fixed] - Merge Android ViewNativeComponent ViewConfig into BaseViewConfig

Reviewed By: elicwhite

Differential Revision: D64570806

fbshipit-source-id: de5c590e935c879e33d59c36ddce1f2481023c19
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Oct 18, 2024
1 parent 797d013 commit 0ba00fc
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 90 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,101 +11,16 @@
import type {
HostComponent,
HostInstance,
PartialViewConfig,
} from '../../Renderer/shims/ReactNativeTypes';

import * as NativeComponentRegistry from '../../NativeComponent/NativeComponentRegistry';
import codegenNativeCommands from '../../Utilities/codegenNativeCommands';
import Platform from '../../Utilities/Platform';
import {type ViewProps as Props} from './ViewPropTypes';

export const __INTERNAL_VIEW_CONFIG: PartialViewConfig =
Platform.OS === 'android'
? {
uiViewClassName: 'RCTView',
validAttributes: {
// ReactClippingViewManager @ReactProps
removeClippedSubviews: true,

// ReactViewManager @ReactProps
accessible: true,
hasTVPreferredFocus: true,
nextFocusDown: true,
nextFocusForward: true,
nextFocusLeft: true,
nextFocusRight: true,
nextFocusUp: true,

borderRadius: true,
borderTopLeftRadius: true,
borderTopRightRadius: true,
borderBottomRightRadius: true,
borderBottomLeftRadius: true,
borderTopStartRadius: true,
borderTopEndRadius: true,
borderBottomStartRadius: true,
borderBottomEndRadius: true,
borderEndEndRadius: true,
borderEndStartRadius: true,
borderStartEndRadius: true,
borderStartStartRadius: true,
borderStyle: true,
hitSlop: true,
pointerEvents: true,
nativeBackgroundAndroid: true,
nativeForegroundAndroid: true,
needsOffscreenAlphaCompositing: true,

borderWidth: true,
borderLeftWidth: true,
borderRightWidth: true,
borderTopWidth: true,
borderBottomWidth: true,
borderStartWidth: true,
borderEndWidth: true,

borderColor: {
process: require('../../StyleSheet/processColor').default,
},
borderLeftColor: {
process: require('../../StyleSheet/processColor').default,
},
borderRightColor: {
process: require('../../StyleSheet/processColor').default,
},
borderTopColor: {
process: require('../../StyleSheet/processColor').default,
},
borderBottomColor: {
process: require('../../StyleSheet/processColor').default,
},
borderStartColor: {
process: require('../../StyleSheet/processColor').default,
},
borderEndColor: {
process: require('../../StyleSheet/processColor').default,
},
borderBlockColor: {
process: require('../../StyleSheet/processColor').default,
},
borderBlockEndColor: {
process: require('../../StyleSheet/processColor').default,
},
borderBlockStartColor: {
process: require('../../StyleSheet/processColor').default,
},
focusable: true,
overflow: true,
backfaceVisibility: true,
experimental_layoutConformance: true,
},
}
: {
uiViewClassName: 'RCTView',
};

const ViewNativeComponent: HostComponent<Props> =
NativeComponentRegistry.get<Props>('RCTView', () => __INTERNAL_VIEW_CONFIG);
NativeComponentRegistry.get<Props>('RCTView', () => ({
uiViewClassName: 'RCTView',
}));

interface NativeCommands {
+hotspotUpdate: (viewRef: HostInstance, x: number, y: number) => void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,71 @@ const validAttributesForNonEventProps = {
style: ReactNativeStyleAttributes,

experimental_layoutConformance: true,

// ReactClippingViewManager @ReactProps
removeClippedSubviews: true,

// ReactViewManager @ReactProps
accessible: true,
hasTVPreferredFocus: true,
nextFocusDown: true,
nextFocusForward: true,
nextFocusLeft: true,
nextFocusRight: true,
nextFocusUp: true,

borderRadius: true,
borderTopLeftRadius: true,
borderTopRightRadius: true,
borderBottomRightRadius: true,
borderBottomLeftRadius: true,
borderTopStartRadius: true,
borderTopEndRadius: true,
borderBottomStartRadius: true,
borderBottomEndRadius: true,
borderEndEndRadius: true,
borderEndStartRadius: true,
borderStartEndRadius: true,
borderStartStartRadius: true,
borderStyle: true,
hitSlop: true,
pointerEvents: true,
nativeBackgroundAndroid: true,
nativeForegroundAndroid: true,
needsOffscreenAlphaCompositing: true,

borderColor: {
process: require('../StyleSheet/processColor').default,
},
borderLeftColor: {
process: require('../StyleSheet/processColor').default,
},
borderRightColor: {
process: require('../StyleSheet/processColor').default,
},
borderTopColor: {
process: require('../StyleSheet/processColor').default,
},
borderBottomColor: {
process: require('../StyleSheet/processColor').default,
},
borderStartColor: {
process: require('../StyleSheet/processColor').default,
},
borderEndColor: {
process: require('../StyleSheet/processColor').default,
},
borderBlockColor: {
process: require('../StyleSheet/processColor').default,
},
borderBlockEndColor: {
process: require('../StyleSheet/processColor').default,
},
borderBlockStartColor: {
process: require('../StyleSheet/processColor').default,
},
focusable: true,
backfaceVisibility: true,
};

// Props for bubbling and direct events
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4071,8 +4071,7 @@ export type AccessibilityValue = $ReadOnly<{|
`;

exports[`public API should not change unintentionally Libraries/Components/View/ViewNativeComponent.js 1`] = `
"declare export const __INTERNAL_VIEW_CONFIG: PartialViewConfig;
declare const ViewNativeComponent: HostComponent<Props>;
"declare const ViewNativeComponent: HostComponent<Props>;
interface NativeCommands {
+hotspotUpdate: (viewRef: HostInstance, x: number, y: number) => void;
+setPressed: (viewRef: HostInstance, pressed: boolean) => void;
Expand Down

0 comments on commit 0ba00fc

Please sign in to comment.