已关闭
fix: 补充close/open链卸载守卫与兜底;panresponderThreshold阈值生效;supportedOrientations朝向约束 #33
fix: 补充close/open链卸载守卫与兜底;panresponderThreshold阈值生效;supportedOrientations朝向约束 #33
已关闭
mazheng创建于 18 天前关闭于 1 天前
7 个文件变更+333-90
@@ -1,3 +1,12 @@
1+# v14.0.2-beta.1
2+ 
3+- fix: 修复关闭弹窗时抛出 TypeError 的问题。为 open/close 链路补充组件卸载守卫(卸载后置空 ref、丢弃迟到的动画回调),并在 animatable 宿主缺失时走无动画兜底,避免 `isTransitioning` 卡在 true 后再也无法打开/关闭弹窗。
4+- fix: `panResponderThreshold` 属性生效。滑动识别改为在累计位移超过阈值后才触发,低于阈值的手势视为点击,弹窗不再位移,`onSwipeStart`/`onSwipeMove` 也不会误触发。
5+- fix: `hideModalContentWhileAnimating` 属性生效。原逻辑依赖 `useNativeDriver` 且依赖弹窗可见期恒为 true 的 `showContent`,导致属性始终不起作用;现改为由进出场过渡自身驱动,取值为 true 时整个动画过程都隐藏弹窗内容,且不再受动画驱动方式限制。
6+- feat: HarmonyOS 上按 `supportedOrientations` 约束弹窗布局尺寸,窗口旋转到未声明支持的方向时,弹窗仍按支持的方向渲染(iOS 原生能力的布局层模拟,不会真正锁定窗口方向)。
7+- fix: 滑动百分比改用真实窗口尺寸计算,修复朝向约束下分母为负、导致遮罩透明度异常与 `onSwipeMove` 回传百分比越界的问题。
8+- chore: 恢复 RN 0.61 分支的 CI 打包流程。补充 `fast:pkg` 脚本先编译再打包、限定 ambient 类型范围与所需 ES 库,并新增兼容类型声明使 TypeScript 3.7 可以构建。
9+ 
1# v14.0.110# v14.0.1
2 11 
3- fix: BackHandler.removeEventListener for compatibility with RN Version 0.77+12- fix: BackHandler.removeEventListener for compatibility with RN Version 0.77+
@@ -8,6 +8,13 @@ This project is based on [react-native-modal@14.0.0](https://github.com/react-na
8 8 
9- [English](https://gitcode.com/OpenHarmony-RN/usage-docs/blob/master/en/react-native-modal.md)9- [English](https://gitcode.com/OpenHarmony-RN/usage-docs/blob/master/en/react-native-modal.md)
10 10 
11+## CI package build
12+ 
13+Run `npm run fast:pkg` to install dependencies, compile TypeScript and create the
14+npm tarball with JavaScript and type declarations in `dist/`.
15+`buildEnv.sh` points CI to the repository root. This package has no native Harmony
16+project, so CI only builds the npm package and does not assemble a HAP or HAR.
17+ 
11## License18## License
12 19 
13This library is licensed under [The MIT License (MIT)](https://github.com/react-native-modal/react-native-modal/blob/master/LICENSE.md)20This library is licensed under [The MIT License (MIT)](https://github.com/react-native-modal/react-native-modal/blob/master/LICENSE.md)
@@ -0,0 +1,5 @@
1+#!/bin/bash
2+# This TypeScript-only library builds its npm package from the repository root.
3+# There is no native Harmony project (build-profile.json5) to assemble.
4+BUILD_PKG_DIR=.
5+BUILD_SAMPLE_DIR=.
@@ -1,6 +1,6 @@
1{1{
2 "name": "@react-native-ohos/react-native-modal",2 "name": "@react-native-ohos/react-native-modal",
3- "version": "14.0.1",3+ "version": "14.0.2-beta.1",
4 "description": "An enhanced React Native modal",4 "description": "An enhanced React Native modal",
5 "main": "dist/index.js",5 "main": "dist/index.js",
6 "types": "dist/index.d.ts",6 "types": "dist/index.d.ts",
@@ -12,6 +12,7 @@
12 "test": "yarn run test:ts && yarn run lint",12 "test": "yarn run test:ts && yarn run lint",
13 "release": "yarn semantic-release",13 "release": "yarn semantic-release",
14 "build": "tsc",14 "build": "tsc",
15+ "fast:pkg": "npm install --legacy-peer-deps --ignore-scripts && npm run build && npm pack",
15 "dev": "tsc --watch",16 "dev": "tsc --watch",
16 "test:ts": "tsc --noEmit"17 "test:ts": "tsc --noEmit"
17 },18 },
@@ -48,9 +48,19 @@ export type OnSwipeCompleteParams = {
48 48 
49type State = {49type State = {
50 showContent: boolean;50 showContent: boolean;
51+ // True from the moment an entrance/exit transition is kicked off until it
52+ // settles. Unlike the isTransitioning instance field this lives in state,
53+ // so toggling it re-renders and hideModalContentWhileAnimating can actually
54+ // take effect.
55+ isAnimatingContent: boolean;
51 isVisible: boolean;56 isVisible: boolean;
52 deviceWidth: number;57 deviceWidth: number;
53 deviceHeight: number;58 deviceHeight: number;
59+ // Real (unconstrained) window size. The backdrop is sized from the real
60+ // window at render time, so these are tracked to detect rotations that do
61+ // not change the orientation-constrained geometry and still re-render.
62+ realWindowWidth: number;
63+ realWindowHeight: number;
54 isSwipeable: boolean;64 isSwipeable: boolean;
55 pan: OrNull<Animated.ValueXY>;65 pan: OrNull<Animated.ValueXY>;
56};66};
@@ -188,9 +198,12 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
188 // device rotation.198 // device rotation.
189 state: State = {199 state: State = {
190 showContent: true,200 showContent: true,
201+ isAnimatingContent: false,
191 isVisible: false,202 isVisible: false,
192 deviceWidth: Dimensions.get('window').width,203 deviceWidth: Dimensions.get('window').width,
193 deviceHeight: Dimensions.get('window').height,204 deviceHeight: Dimensions.get('window').height,
205+ realWindowWidth: Dimensions.get('window').width,
206+ realWindowHeight: Dimensions.get('window').height,
194 isSwipeable: !!this.props.swipeDirection,207 isSwipeable: !!this.props.swipeDirection,
195 pan: null,208 pan: null,
196 };209 };
@@ -198,9 +211,15 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
198 isTransitioning = false;211 isTransitioning = false;
199 inSwipeClosingState = false;212 inSwipeClosingState = false;
200 currentSwipingDirection: OrNull<Direction> = null;213 currentSwipingDirection: OrNull<Direction> = null;
214+ // Set in componentWillUnmount so that late asynchronous callbacks
215+ // (animate() promise resolutions) can bail out instead of touching an
216+ // unmounted instance (prevents setState-after-unmount and lifecycle
217+ // callbacks firing after the modal is gone).
218+ isUnmounted = false;
201 219 
202 animationIn: string;220 animationIn: string;
203 animationOut: string;221 animationOut: string;
222+ backdropAnimated: Animated.Value;
204 backdropRef: any;223 backdropRef: any;
205 contentRef: any;224 contentRef: any;
206 panResponder: OrNull<PanResponderInstance> = null;225 panResponder: OrNull<PanResponderInstance> = null;
@@ -217,6 +236,7 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
217 236 
218 this.animationIn = animationIn;237 this.animationIn = animationIn;
219 this.animationOut = animationOut;238 this.animationOut = animationOut;
239+ this.backdropAnimated = new Animated.Value(0);
220 240 
221 if (this.state.isSwipeable) {241 if (this.state.isSwipeable) {
222 this.state = {242 this.state = {
@@ -230,13 +250,17 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
230 ...this.state,250 ...this.state,
231 isVisible: true,251 isVisible: true,
232 showContent: true,252 showContent: true,
253+ isAnimatingContent: true,
233 };254 };
234 }255 }
235 }256 }
236 257 
237 static getDerivedStateFromProps(nextProps: Readonly<ModalProps>, state: State) {258 static getDerivedStateFromProps(nextProps: Readonly<ModalProps>, state: State) {
238 if (!state.isVisible && nextProps.isVisible) {259 if (!state.isVisible && nextProps.isVisible) {
239- return { isVisible: true, showContent: true };260+ // isAnimatingContent is raised here rather than in open() so that the
261+ // very first render of a hidden-while-animating modal already omits the
262+ // content, instead of painting it for one frame.
263+ return {isVisible: true, showContent: true, isAnimatingContent: true};
240 }264 }
241 return null;265 return null;
242 }266 }
@@ -267,6 +291,7 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
267 };291 };
268 292 
269 componentWillUnmount() {293 componentWillUnmount() {
294+ this.isUnmounted = true;
270 if (this.isVersionGreaterOrEqual(0, 77, 0)) {295 if (this.isVersionGreaterOrEqual(0, 77, 0)) {
271 if (this.backListener) {296 if (this.backListener) {
272 this.backListener.remove();297 this.backListener.remove();
@@ -284,6 +309,13 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
284 InteractionManager.clearInteractionHandle(this.interactionHandle);309 InteractionManager.clearInteractionHandle(this.interactionHandle);
285 this.interactionHandle = null;310 this.interactionHandle = null;
286 }311 }
312+ // Stop any running backdrop animation
313+ if (this.backdropAnimated && this.backdropAnimated.stopAnimation) {
314+ this.backdropAnimated.stopAnimation();
315+ }
316+ // Drop refs so any callback that still fires after unmount is a no-op
317+ this.contentRef = null;
318+ this.backdropRef = null;
287 }319 }
288 320 
289 componentDidUpdate(prevProps: ModalProps) {321 componentDidUpdate(prevProps: ModalProps) {
@@ -299,15 +331,13 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
299 this.animationIn = animationIn;331 this.animationIn = animationIn;
300 this.animationOut = animationOut;332 this.animationOut = animationOut;
301 }333 }
302- // If backdrop opacity has been changed then make sure to update it334+ // If backdrop opacity has been changed then animate the Animated.Value
303- if (335+ if (this.props.backdropOpacity !== prevProps.backdropOpacity) {
304- this.props.backdropOpacity !== prevProps.backdropOpacity &&336+ Animated.timing(this.backdropAnimated, {
305- this.backdropRef337+ toValue: this.props.backdropOpacity,
306- ) {338+ duration: this.props.backdropTransitionInTiming,
307- this.backdropRef.transitionTo(339+ useNativeDriver: this.props.useNativeDriverForBackdrop === true,
308- { opacity: this.props.backdropOpacity },340+ }).start();
309- this.props.backdropTransitionInTiming,
310- );
311 }341 }
312 // On modal open request, we slide the view up and fade in the backdrop342 // On modal open request, we slide the view up and fade in the backdrop
313 if (this.props.isVisible && !prevProps.isVisible) {343 if (this.props.isVisible && !prevProps.isVisible) {
@@ -319,6 +349,19 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
319 }349 }
320 getDeviceHeight = () => this.props.deviceHeight || this.state.deviceHeight;350 getDeviceHeight = () => this.props.deviceHeight || this.state.deviceHeight;
321 getDeviceWidth = () => this.props.deviceWidth || this.state.deviceWidth;351 getDeviceWidth = () => this.props.deviceWidth || this.state.deviceWidth;
352+ // Swipe math must be measured in real window space: gestureState.y0/moveY
353+ // (and x0/moveX) are reported in real screen coordinates, while
354+ // state.deviceHeight/deviceWidth may hold the orientation-constrained
355+ // geometry produced by constrainToSupportedOrientation() — width and height
356+ // swapped. Measuring a real-space gesture against the swapped extent gives a
357+ // wrong percentage, and a negative denominator once the touch starts beyond
358+ // the swapped edge (e.g. y0 = 600 on a 400x800 portrait window constrained to
359+ // landscape: 400 - 600 = -200), which pushes the backdrop opacity factor
360+ // above 1 and hands onSwipeMove an out-of-range percentage.
361+ getGestureDeviceHeight = () =>
362+ this.props.deviceHeight || Dimensions.get('window').height;
363+ getGestureDeviceWidth = () =>
364+ this.props.deviceWidth || Dimensions.get('window').width;
322 onBackButtonPress = () => {365 onBackButtonPress = () => {
323 if (this.props.onBackButtonPress && this.props.isVisible) {366 if (this.props.onBackButtonPress && this.props.isVisible) {
324 this.props.onBackButtonPress();367 this.props.onBackButtonPress();
@@ -379,13 +422,16 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
379 ) {422 ) {
380 return false; // user needs to be able to scroll content back up423 return false; // user needs to be able to scroll content back up
381 }424 }
382- if (this.props.onSwipeStart) {
383- this.props.onSwipeStart(gestureState);
384- }
385 425 
386 // Cleared so that onPanResponderMove can wait to have some delta426 // Cleared so that onPanResponderMove can wait to have some delta
387 // to work with427 // to work with
388 this.currentSwipingDirection = null;428 this.currentSwipingDirection = null;
429+ // NOTE: onSwipeStart is intentionally NOT fired here. At touch-down
430+ // there is no movement yet, so the swipe has not started. It fires
431+ // in onPanResponderMove once the accumulated delta crosses
432+ // panResponderThreshold (and in onMoveShouldSetPanResponder for the
433+ // scrollable-content negotiation path), keeping
434+ // panResponderThreshold meaningful for swipe recognition.
389 return true;435 return true;
390 },436 },
391 onPanResponderMove: (evt, gestureState) => {437 onPanResponderMove: (evt, gestureState) => {
@@ -395,6 +441,19 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
395 if (gestureState.dx === 0 && gestureState.dy === 0) {441 if (gestureState.dx === 0 && gestureState.dy === 0) {
396 return;442 return;
397 }443 }
444+ // The swipe is only recognized once the accumulated delta reaches
445+ // panResponderThreshold. Below the threshold the gesture is treated
446+ // as a tap / inner-content gesture so the modal does not move and
447+ // onSwipeStart/onSwipeMove are not emitted.
448+ if (
449+ Math.abs(gestureState.dx) < this.props.panResponderThreshold &&
450+ Math.abs(gestureState.dy) < this.props.panResponderThreshold
451+ ) {
452+ return;
453+ }
454+ if (this.props.onSwipeStart) {
455+ this.props.onSwipeStart(gestureState);
456+ }
398 457 
399 this.currentSwipingDirection = this.getSwipingDirection(gestureState);458 this.currentSwipingDirection = this.getSwipingDirection(gestureState);
400 animEvt = this.createAnimationEventForSwipe();459 animEvt = this.createAnimationEventForSwipe();
@@ -405,10 +464,14 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
405 const newOpacityFactor =464 const newOpacityFactor =
406 1 - this.calcDistancePercentage(gestureState);465 1 - this.calcDistancePercentage(gestureState);
407 466 
408- this.backdropRef &&467+ // Update backdrop opacity immediately during swipe. setValue() is
409- this.backdropRef.transitionTo({468+ // synchronous and works regardless of useNativeDriver setting —
410- opacity: this.props.backdropOpacity * newOpacityFactor,469+ // it directly sets the Animated.Value without scheduling an
411- });470+ // animation (transitionTo would start a new spring per move event
471+ // and lag behind the finger).
472+ this.backdropAnimated.setValue(
473+ this.props.backdropOpacity * newOpacityFactor,
474+ );
412 475 
413 animEvt!(evt, gestureState);476 animEvt!(evt, gestureState);
414 477 
@@ -461,14 +524,20 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
461 }524 }
462 525 
463 //Reset backdrop opacity and modal position526 //Reset backdrop opacity and modal position
464- if (this.props.onSwipeCancel) {527+ // onSwipeCancel pairs with onSwipeStart, which fires only after the
528+ // accumulated delta crosses panResponderThreshold — skip it for
529+ // pure taps where the swipe never started.
530+ if (this.currentSwipingDirection && this.props.onSwipeCancel) {
465 this.props.onSwipeCancel(gestureState);531 this.props.onSwipeCancel(gestureState);
466 }532 }
467 533 
468 if (this.backdropRef) {534 if (this.backdropRef) {
469- this.backdropRef.transitionTo({535+ // Reset backdrop opacity with a smooth animation
470- opacity: this.props.backdropOpacity,536+ Animated.timing(this.backdropAnimated, {
471- });537+ toValue: this.props.backdropOpacity,
538+ duration: this.props.backdropTransitionInTiming,
539+ useNativeDriver: this.props.useNativeDriverForBackdrop === true,
540+ }).start();
472 }541 }
473 542 
474 Animated.spring(this.state.pan!, {543 Animated.spring(this.state.pan!, {
@@ -517,8 +586,7 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
517 case 'down':586 case 'down':
518 return (587 return (
519 (gestureState.moveY - gestureState.y0) /588 (gestureState.moveY - gestureState.y0) /
520- ((this.props.deviceHeight || this.state.deviceHeight) -589+ (this.getGestureDeviceHeight() - gestureState.y0)
521- gestureState.y0)
522 );590 );
523 case 'up':591 case 'up':
524 return reversePercentage(gestureState.moveY / gestureState.y0);592 return reversePercentage(gestureState.moveY / gestureState.y0);
@@ -527,7 +595,7 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
527 case 'right':595 case 'right':
528 return (596 return (
529 (gestureState.moveX - gestureState.x0) /597 (gestureState.moveX - gestureState.x0) /
530- ((this.props.deviceWidth || this.state.deviceWidth) - gestureState.x0)598+ (this.getGestureDeviceWidth() - gestureState.x0)
531 );599 );
532 600 
533 default:601 default:
@@ -590,31 +658,141 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
590 return false;658 return false;
591 };659 };
592 660 
661+ // Emulates the iOS-only `supportedOrientations` contract on HarmonyOS,
662+ // where the RNOH Modal host has no native orientation support: if the
663+ // window rotates to an orientation that is not part of
664+ // props.supportedOrientations, the modal keeps being laid out in a
665+ // supported orientation (portrait box for portrait-only sets, landscape
666+ // box for landscape-only sets) instead of following the rotation.
667+ constrainToSupportedOrientation = (width: number, height: number) => {
668+ // Harmony-only: on iOS supportedOrientations is enforced natively by the
669+ // Modal presentation, and on Android the prop is a documented no-op, so
670+ // the JS emulation must not alter their behavior.
671+ if (!['harmony'].includes(Platform.OS)) {
672+ return {width, height};
673+ }
674+ const supported = this.props.supportedOrientations;
675+ if (!supported || supported.length === 0) {
676+ return {width, height};
677+ }
678+ const orientation = width > height ? 'landscape' : 'portrait';
679+ const isSupported =
680+ supported.includes(orientation as Orientation) ||
681+ (orientation === 'portrait' &&
682+ supported.includes('portrait-upside-down')) ||
683+ (orientation === 'landscape' &&
684+ (supported.includes('landscape-left') ||
685+ supported.includes('landscape-right')));
686+ if (isSupported) {
687+ return {width, height};
688+ }
689+ // Unsupported rotation: fall back to the opposite (supported) geometry.
690+ return {width: height, height: width};
cpf-manager
cpf-managercpf-manager9 天前

【AI-Review】【一般】【基础代码问题】【代码逻辑错误】orientation约束导致backdrop无法覆盖全屏

● 问题:constrainToSupportedOrientation 在方向不支持时交换宽高(line 659: return {width: height, height: width}),交换后的尺寸经 constrainDimensionsToSupportedOrientation(line 675)和 handleDimensionsUpdate(line 694)写入 state.deviceWidth/deviceHeight。而 makeBackdrop(line 867-868)使用 getDeviceWidth()/getDeviceHeight() 设置 backdrop 的宽高,返回的正是约束后的 state 值。styles.backdrop 虽设置了 top:0, bottom:0, left:0, right:0,但 backdropComputedStyle 中的 width/height 覆盖了拉伸行为,backdrop 实际尺寸为交换后的值。

触发路径:用户设置 supportedOrientations={['portrait']}(仅竖屏)→ 设备处于横屏(如 800×400)→ open() 调用 constrainDimensionsToSupportedOrientation()constrainToSupportedOrientation(800, 400) 判定 orientation='landscape',isSupported=false → 返回 {width:400, height:800}(交换)→ state 更新为 deviceWidth=400, deviceHeight=800 → makeBackdrop 渲染 backdrop 为 400×800 → 在 800×400 屏幕上仅覆盖左上 400×400,右侧 400 像素未被覆盖。

● 影响:在 HarmonyOS 上,当 supportedOrientations 不包含当前设备方向时,backdrop 无法覆盖全屏,用户可以看到并操作 backdrop 外的区域(Modal 为 transparent={true}),破坏模态框遮挡背景的设计意图。

● 建议:backdrop 应使用实际屏幕尺寸(Dimensions.get('window'))而非约束后的 state 值,确保全屏覆盖。可将 makeBackdrop 中的 width/height 改为直接使用 Dimensions.get('window').width/height,或将约束后尺寸仅用于内容布局,不影响 backdrop 全屏覆盖。

likedislike
691+ };
692+ 
693+ constrainDimensionsToSupportedOrientation = () => {
694+ if (this.props.deviceHeight || this.props.deviceWidth) {
695+ return;
696+ }
697+ const windowDimensions = Dimensions.get('window');
698+ const {width, height} = this.constrainToSupportedOrientation(
699+ windowDimensions.width,
700+ windowDimensions.height,
701+ );
702+ if (
703+ width !== this.state.deviceWidth ||
704+ height !== this.state.deviceHeight
705+ ) {
706+ this.setState({deviceWidth: width, deviceHeight: height});
707+ }
708+ };
709+ 
593 handleDimensionsUpdate = () => {710 handleDimensionsUpdate = () => {
594 if (!this.props.deviceHeight && !this.props.deviceWidth) {711 if (!this.props.deviceHeight && !this.props.deviceWidth) {
595 // Here we update the device dimensions in the state if the layout changed712 // Here we update the device dimensions in the state if the layout changed
596 // (triggering a render)713 // (triggering a render)
597- const deviceWidth = Dimensions.get('window').width;714+ const windowDimensions = Dimensions.get('window');
598- const deviceHeight = Dimensions.get('window').height;715+ const {width, height} = this.constrainToSupportedOrientation(
716+ windowDimensions.width,
717+ windowDimensions.height,
718+ );
719+ const deviceWidth = width;
720+ const deviceHeight = height;
599 if (721 if (
600 deviceWidth !== this.state.deviceWidth ||722 deviceWidth !== this.state.deviceWidth ||
601- deviceHeight !== this.state.deviceHeight723+ deviceHeight !== this.state.deviceHeight ||
724+ // Rotating to an unsupported orientation keeps the constrained
725+ // geometry unchanged, but the backdrop is sized from the real
726+ // window — re-render whenever the real window changed too.
727+ windowDimensions.width !== this.state.realWindowWidth ||
728+ windowDimensions.height !== this.state.realWindowHeight
602 ) {729 ) {
603- this.setState({ deviceWidth, deviceHeight });730+ this.setState({
731+ deviceWidth,
732+ deviceHeight,
733+ realWindowWidth: windowDimensions.width,
734+ realWindowHeight: windowDimensions.height,
735+ });
604 }736 }
605 }737 }
606 };738 };
607 739 
740+ // Mirrors isTransitioning into state so that render() can honor
741+ // hideModalContentWhileAnimating. Guarded against redundant updates because
742+ // the prop-driven open path already raises the flag in
743+ // getDerivedStateFromProps.
744+ setAnimatingContent = (isAnimatingContent: boolean) => {
745+ if (
746+ this.isUnmounted ||
747+ this.state.isAnimatingContent === isAnimatingContent
748+ ) {
749+ return;
750+ }
751+ this.setState({isAnimatingContent});
752+ };
753+ 
754+ // Completes the open sequence without any content animation. Used as a
755+ // fallback when the animatable host is (or became) unavailable mid-open,
756+ // so the modal never gets stuck with isTransitioning === true (which
757+ // would block all subsequent open/close calls). Also serves as the
758+ // promise-resolution handler of a normal open.
759+ completeOpenWithoutAnimation = () => {
760+ this.isTransitioning = false;
761+ if (this.interactionHandle) {
762+ InteractionManager.clearInteractionHandle(this.interactionHandle);
763+ this.interactionHandle = null;
764+ }
765+ if (this.isUnmounted) {
766+ return;
767+ }
768+ if (!this.props.isVisible) {
769+ this.close();
770+ } else {
771+ this.setAnimatingContent(false);
772+ this.props.onModalShow();
773+ }
774+ };
775+ 
608 open = () => {776 open = () => {
609 if (this.isTransitioning) {777 if (this.isTransitioning) {
610 return;778 return;
611 }779 }
612 this.isTransitioning = true;780 this.isTransitioning = true;
781+ this.setAnimatingContent(true);
782+ // Present the modal in a supported orientation (HarmonyOS emulation of
783+ // the iOS-only supportedOrientations contract) when the window is
784+ // already rotated to an unsupported orientation.
785+ this.constrainDimensionsToSupportedOrientation();
613 if (this.backdropRef) {786 if (this.backdropRef) {
614- this.backdropRef.transitionTo(787+ // Rewind the backdrop to fully transparent before fading in, so the
615- { opacity: this.props.backdropOpacity },788+ // fade-in always starts from a known value.
616- this.props.backdropTransitionInTiming,789+ this.backdropAnimated.stopAnimation();
617- );790+ this.backdropAnimated.setValue(0);
791+ Animated.timing(this.backdropAnimated, {
792+ toValue: this.props.backdropOpacity,
793+ duration: this.props.backdropTransitionInTiming,
794+ useNativeDriver: this.props.useNativeDriverForBackdrop === true,
795+ }).start();
618 }796 }
619 797 
620 // This is for resetting the pan position,otherwise the modal gets stuck798 // This is for resetting the pan position,otherwise the modal gets stuck
@@ -632,17 +810,16 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
632 this.contentRef810 this.contentRef
633 .animate(this.animationIn, this.props.animationInTiming)811 .animate(this.animationIn, this.props.animationInTiming)
634 .then(() => {812 .then(() => {
635- this.isTransitioning = false;813+ if (this.isUnmounted) {
636- if (this.interactionHandle) {814+ return;
637- InteractionManager.clearInteractionHandle(this.interactionHandle);
638- this.interactionHandle = null;
639- }
640- if (!this.props.isVisible) {
641- this.close();
642- } else {
643- this.props.onModalShow();
644 }815 }
816+ this.completeOpenWithoutAnimation();
645 });817 });
818+ } else {
819+ // Content host unavailable (e.g. the Modal host dismissed the content
820+ // or the component is unmounting). Complete the open sequence without
821+ // animation so the modal does not stay stuck in isTransitioning state.
822+ this.completeOpenWithoutAnimation();
cpf-manager
cpf-managercpf-manager17 天前

【AI-Review】【一般】【基础代码问题】【代码逻辑错误】open() fallback 路径缺少 onModalWillShow 回调,违反生命周期契约

● 问题: open() 的 else 分支(contentRef 为 null 时,line 744-749)直接调用 this.completeOpenWithoutAnimation()(line 748),该方法在 isVisible 为 true 时直接触发 this.props.onModalShow()(line 704),但 else 分支未先调用 this.props.onModalWillShow。对比 close() 的 else 分支(line 792-798)在 this.finalizeClose()(line 797)之前正确补齐了 this.props.onModalWillHide && this.props.onModalWillHide()(line 796),open 与 close 的 fallback 路径存在不对称性。

触发路径: Modal host 提前 dismiss 内容导致 this.contentRef 为 null(如 RNOH Modal 原生窗口在内容挂载前被关闭,或组件处于卸载过程中但 isUnmounted 尚未置 true)→ 用户设置 isVisible=truecomponentDidUpdate(line 321)调用 open()isTransitioning 为 false,进入 → this.contentRef 为 null → 进入 else 分支(line 744)→ completeOpenWithoutAnimation()(line 748)→ isUnmounted 为 false → isVisible 为 true → this.props.onModalShow()(line 704)触发,但 onModalWillShow 从未触发。

● 影响: 一般。依赖 onModalWillShow 回调的调用方(如在该回调中准备 UI、启动并行动画、埋点上报)在 fallback 路径下会丢失该回调,仅收到 onModalShow,违反 onModalWillShow → onModalShow 的生命周期契约,可能导致调用方前置逻辑未执行。

● 建议: 在 else 分支的 this.completeOpenWithoutAnimation() 之前补齐 onModalWillShow 调用,与 close() 的 fallback 路径保持对称: } else { this.props.onModalWillShow && this.props.onModalWillShow(); this.completeOpenWithoutAnimation(); }

likedislike
646 }823 }
647 };824 };
648 825 
@@ -651,11 +828,16 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
651 return;828 return;
652 }829 }
653 this.isTransitioning = true;830 this.isTransitioning = true;
831+ this.setAnimatingContent(true);
654 if (this.backdropRef) {832 if (this.backdropRef) {
655- this.backdropRef.transitionTo(833+ if (this.backdropAnimated && this.backdropAnimated.stopAnimation) {
656- { opacity: 0 },834+ this.backdropAnimated.stopAnimation();
657- this.props.backdropTransitionOutTiming,835+ }
658- );836+ Animated.timing(this.backdropAnimated, {
837+ toValue: 0,
838+ duration: this.props.backdropTransitionOutTiming,
839+ useNativeDriver: this.props.useNativeDriverForBackdrop === true,
840+ }).start();
659 }841 }
660 842 
661 let animationOut = this.animationOut;843 let animationOut = this.animationOut;
@@ -681,31 +863,53 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
681 this.contentRef863 this.contentRef
682 .animate(animationOut, this.props.animationOutTiming)864 .animate(animationOut, this.props.animationOutTiming)
683 .then(() => {865 .then(() => {
684- this.isTransitioning = false;866+ if (this.isUnmounted) {
685- if (this.interactionHandle) {867+ return;
686- InteractionManager.clearInteractionHandle(this.interactionHandle);
687- this.interactionHandle = null;
688- }
689- if (this.props.isVisible) {
690- this.open();
691- } else {
692- this.setState(
693- {
694- showContent: false,
695- },
696- () => {
697- this.setState(
698- {
699- isVisible: false,
700- },
701- () => {
702- this.props.onModalHide();
703- },
704- );
705- },
706- );
707 }868 }
869+ this.finalizeClose();
708 });870 });
871+ } else {
872+ // Content host is already gone (unmounted or never mounted): nothing to
873+ // animate. Finalize directly so isTransitioning is not left stuck true
874+ // (which would block every future open/close) and onModalHide still runs.
875+ this.props.onModalWillHide && this.props.onModalWillHide();
876+ this.finalizeClose();
877+ }
878+ };
879+ 
880+ // Shared tail of the close sequence, used by the animate() promise
881+ // resolution and the no-content fallback path.
882+ finalizeClose = () => {
883+ if (this.isUnmounted) {
884+ return;
885+ }
886+ this.isTransitioning = false;
887+ if (this.interactionHandle) {
888+ InteractionManager.clearInteractionHandle(this.interactionHandle);
889+ this.interactionHandle = null;
890+ }
891+ if (this.props.isVisible) {
892+ this.open();
893+ } else {
894+ this.setState(
895+ {
896+ showContent: false,
897+ },
898+ () => {
899+ // isAnimatingContent must drop together with isVisible: resetting
900+ // it earlier re-renders the (hidden) children for one commit while
901+ // the modal is still visible, flashing content before the hide.
902+ this.setState(
903+ {
904+ isVisible: false,
905+ isAnimatingContent: false,
906+ },
907+ () => {
908+ this.props.onModalHide();
909+ },
910+ );
911+ },
912+ );
709 }913 }
710 };914 };
711 makeBackdrop = () => {915 makeBackdrop = () => {
@@ -723,16 +927,20 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
723 const {927 const {
724 customBackdrop,928 customBackdrop,
725 backdropColor,929 backdropColor,
726- useNativeDriver,
727- useNativeDriverForBackdrop,
728 onBackdropPress,930 onBackdropPress,
729 } = this.props;931 } = this.props;
730 const hasCustomBackdrop = !!this.props.customBackdrop;932 const hasCustomBackdrop = !!this.props.customBackdrop;
731 933 
732 const backdropComputedStyle = [934 const backdropComputedStyle = [
733 {935 {
734- width: this.getDeviceWidth(),936+ // The backdrop must cover the real window: getDeviceWidth()/
735- height: this.getDeviceHeight(),937+ // getDeviceHeight() may return the orientation-constrained
938+ // (swapped) geometry produced by constrainToSupportedOrientation(),
939+ // and explicit width/height overrides the inset-based stretch in
940+ // styles.backdrop — using the constrained values would leave part
941+ // of the screen uncovered.
942+ width: this.getGestureDeviceWidth(),
943+ height: this.getGestureDeviceHeight(),
736 backgroundColor:944 backgroundColor:
737 this.state.showContent && !hasCustomBackdrop945 this.state.showContent && !hasCustomBackdrop
738 ? backdropColor946 ? backdropColor
@@ -741,16 +949,15 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
741 ];949 ];
742 950 
743 const backdropWrapper = (951 const backdropWrapper = (
744- <animatable.View952+ <Animated.View
745- ref={ref => (this.backdropRef = ref)}953+ ref={(ref: any) => (this.backdropRef = ref)}
746- useNativeDriver={954+ style={[
747- useNativeDriverForBackdrop !== undefined955+ styles.backdrop,
748- ? useNativeDriverForBackdrop956+ backdropComputedStyle,
749- : useNativeDriver957+ {opacity: this.backdropAnimated},
750- }958+ ]}>
751- style={[styles.backdrop, backdropComputedStyle]}>
752 {hasCustomBackdrop && customBackdrop}959 {hasCustomBackdrop && customBackdrop}
753- </animatable.View>960+ </Animated.View>
754 );961 );
755 962 
756 if (hasCustomBackdrop) {963 if (hasCustomBackdrop) {
@@ -803,11 +1010,13 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
803 panHandlers = { ...this.panResponder!.panHandlers };1010 panHandlers = { ...this.panResponder!.panHandlers };
804 1011 
805 if (useNativeDriver) {1012 if (useNativeDriver) {
806- const translateY=this.state.pan.y._value1013+ // getTranslateTransform() keeps the Animated.ValueXY nodes in the
807- const translateX=this.state.pan.x._value1014+ // transform so the (JS-driven) pan updates move the content live.
808- 1015+ // Reading pan._value here would freeze the transform at the values
1016+ // captured at render time (Animated.Value changes do not re-render),
1017+ // so the content would never follow the finger.
809 panPosition = {1018 panPosition = {
810- transform: [{"translateX":Number(translateX)},{"translateY":Number(translateY)}],1019+ transform: this.state.pan!.getTranslateTransform(),
811 };1020 };
812 } else {1021 } else {
813 panPosition = this.state.pan!.getLayout();1022 panPosition = this.state.pan!.getLayout();
@@ -815,11 +1024,13 @@ export class ReactNativeModal extends React.Component<ModalProps, State> {
815 }1024 }
816 1025 
817 // The user might decide not to show the modal while it is animating1026 // The user might decide not to show the modal while it is animating
818- // to enhance performance.1027+ // to enhance performance. Upstream gated this on useNativeDriver and on
1028+ // showContent, which is true for the whole time the modal is visible, so
1029+ // the prop never had any effect. It is now driven by the transition
1030+ // itself and honored for both animation drivers.
819 const _children =1031 const _children =
820 this.props.hideModalContentWhileAnimating &&1032 this.props.hideModalContentWhileAnimating &&
821- this.props.useNativeDriver &&1033+ this.state.isAnimatingContent ? (
822- !this.state.showContent ? (
823 <animatable.View />1034 <animatable.View />
824 ) : (1035 ) : (
825 children1036 children
@@ -0,0 +1,9 @@
1+import 'react-native';
2+ 
3+declare module 'react-native' {
4+ export const ReactNativeVersion: {
5+ major: number;
6+ minor: number;
7+ patch: number;
8+ };
9+}
@@ -7,7 +7,8 @@
7 "jest.config.js"7 "jest.config.js"
8 ],8 ],
9 "compilerOptions": {9 "compilerOptions": {
10- "lib": ["es5", "es6", "esnext.asynciterable"],10+ "lib": ["es2018", "esnext.bigint"],
11+ "types": ["react", "react-native"],
11 "allowSyntheticDefaultImports": false,12 "allowSyntheticDefaultImports": false,
12 "esModuleInterop": false,13 "esModuleInterop": false,
13 "jsx": "react",14 "jsx": "react",