Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
16 changes: 13 additions & 3 deletions src/components/center-popup/center-popup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { renderToContainer } from '../../utils/render-to-container'
import { ShouldRender } from '../../utils/should-render'
import { useInnerVisible } from '../../utils/use-inner-visible'
import { useLockScroll } from '../../utils/use-lock-scroll'
import { useSpringVisibility } from '../../utils/use-spring-visibility'
import { mergeProps } from '../../utils/with-default-props'
import { withStopPropagation } from '../../utils/with-stop-propagation'
import { useConfig } from '../config-provider'
Expand Down Expand Up @@ -42,6 +43,15 @@ export const CenterPopup: FC<CenterPopupProps> = props => {
const mergedProps = mergeProps(defaultProps, componentConfig, props)

const unmountedRef = useUnmountedRef()
const [active, setActive] = useState(mergedProps.visible)
const { shouldCallAfterClose } = useSpringVisibility({
visible: mergedProps.visible,
active,
setActive,
afterClose: mergedProps.afterClose,
unmountedRef,
})

const style = useSpring({
scale: mergedProps.visible ? 1 : 0.8,
opacity: mergedProps.visible ? 1 : 0,
Expand All @@ -53,16 +63,16 @@ export const CenterPopup: FC<CenterPopupProps> = props => {
},
onRest: () => {
if (unmountedRef.current) return
setActive(mergedProps.visible)
if (mergedProps.visible) {
setActive(true)
mergedProps.afterShow?.()
} else {
} else if (shouldCallAfterClose()) {
setActive(false)
mergedProps.afterClose?.()
}
},
})

const [active, setActive] = useState(mergedProps.visible)
useIsomorphicLayoutEffect(() => {
if (mergedProps.visible) {
setActive(true)
Expand Down
14 changes: 12 additions & 2 deletions src/components/mask/mask.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
PropagationEvent,
withStopPropagation,
} from '../../utils/with-stop-propagation'
import { useSpringVisibility } from '../../utils/use-spring-visibility'

const classPrefix = `adm-mask`

Expand Down Expand Up @@ -71,6 +72,14 @@ export const Mask: FC<MaskProps> = p => {
const [active, setActive] = useState(props.visible)

const unmountedRef = useUnmountedRef()
const { shouldCallAfterClose } = useSpringVisibility({
visible: props.visible,
active,
setActive,
afterClose: props.afterClose,
unmountedRef,
})

const { opacity } = useSpring({
opacity: props.visible ? 1 : 0,
config: {
Expand All @@ -85,10 +94,11 @@ export const Mask: FC<MaskProps> = p => {
},
onRest: () => {
if (unmountedRef.current) return
setActive(props.visible)
if (props.visible) {
setActive(true)
props.afterShow?.()
} else {
} else if (shouldCallAfterClose()) {
setActive(false)
props.afterClose?.()
}
},
Expand Down
16 changes: 13 additions & 3 deletions src/components/popup/popup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { defaultPopupBaseProps, PopupBaseProps } from './popup-base-props'
import { useInnerVisible } from '../../utils/use-inner-visible'
import { useConfig } from '../config-provider'
import { useDrag } from '@use-gesture/react'
import { useSpringVisibility } from '../../utils/use-spring-visibility'

const classPrefix = `adm-popup`

Expand Down Expand Up @@ -44,13 +45,21 @@ export const Popup: FC<PopupProps> = p => {
const ref = useRef<HTMLDivElement>(null)
useLockScroll(ref, props.disableBodyScroll && active ? 'strict' : false)

const unmountedRef = useUnmountedRef()
const { shouldCallAfterClose } = useSpringVisibility({
visible: props.visible,
active,
setActive,
afterClose: props.afterClose,
unmountedRef,
})

useIsomorphicLayoutEffect(() => {
if (props.visible) {
setActive(true)
}
}, [props.visible])

const unmountedRef = useUnmountedRef()
const { percent } = useSpring({
percent: props.visible ? 0 : 100,
config: {
Expand All @@ -61,10 +70,11 @@ export const Popup: FC<PopupProps> = p => {
},
onRest: () => {
if (unmountedRef.current) return
setActive(props.visible)
if (props.visible) {
setActive(true)
props.afterShow?.()
} else {
} else if (shouldCallAfterClose()) {
setActive(false)
props.afterClose?.()
}
},
Expand Down
190 changes: 190 additions & 0 deletions src/utils/tests/use-spring-visibility.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
import { renderHook, act } from '@testing-library/react'
import { useSpringVisibility } from '../use-spring-visibility'

describe('useSpringVisibility', () => {
const mockSetActive = jest.fn()
const mockAfterClose = jest.fn()
const mockUnmountedRef = { current: false }
const originalVisibilityState = Object.getOwnPropertyDescriptor(
document,
'visibilityState'
)

beforeEach(() => {
jest.clearAllMocks()
mockUnmountedRef.current = false
})

afterEach(() => {
if (originalVisibilityState) {
Object.defineProperty(
document,
'visibilityState',
originalVisibilityState
)
}
})

it('should call setActive(false) and afterClose when page becomes visible after close while hidden', () => {
const { result } = renderHook(() =>
useSpringVisibility({
visible: false,
active: true,
setActive: mockSetActive,
afterClose: mockAfterClose,
unmountedRef: mockUnmountedRef,
})
)

// Simulate page becoming visible (visibilitychange event)
Object.defineProperty(document, 'visibilityState', {
value: 'visible',
writable: true,
})
act(() => {
document.dispatchEvent(new Event('visibilitychange'))
})

expect(mockSetActive).toHaveBeenCalledWith(false)
expect(mockAfterClose).toHaveBeenCalledTimes(1)
})

it('should not call afterClose when page is already visible and active matches visible', () => {
renderHook(() =>
useSpringVisibility({
visible: false,
active: false,
setActive: mockSetActive,
afterClose: mockAfterClose,
unmountedRef: mockUnmountedRef,
})
)

Object.defineProperty(document, 'visibilityState', {
value: 'visible',
writable: true,
})
act(() => {
document.dispatchEvent(new Event('visibilitychange'))
})

expect(mockSetActive).not.toHaveBeenCalled()
expect(mockAfterClose).not.toHaveBeenCalled()
})

it('should not call afterClose when component is unmounted', () => {
mockUnmountedRef.current = true

renderHook(() =>
useSpringVisibility({
visible: false,
active: true,
setActive: mockSetActive,
afterClose: mockAfterClose,
unmountedRef: mockUnmountedRef,
})
)

Object.defineProperty(document, 'visibilityState', {
value: 'visible',
writable: true,
})
act(() => {
document.dispatchEvent(new Event('visibilitychange'))
})

expect(mockSetActive).not.toHaveBeenCalled()
expect(mockAfterClose).not.toHaveBeenCalled()
})

it('shouldCallAfterClose should prevent double-calling afterClose', () => {
const { result } = renderHook(() =>
useSpringVisibility({
visible: false,
active: true,
setActive: mockSetActive,
afterClose: mockAfterClose,
unmountedRef: mockUnmountedRef,
})
)

// Simulate visibilitychange handler calling afterClose first
Object.defineProperty(document, 'visibilityState', {
value: 'visible',
writable: true,
})
act(() => {
document.dispatchEvent(new Event('visibilitychange'))
})

expect(mockAfterClose).toHaveBeenCalledTimes(1)

// Now onRest fires later - shouldCallAfterClose should return false
expect(result.current.shouldCallAfterClose()).toBe(false)
})

it('shouldCallAfterClose should return true when afterClose has not been called', () => {
const { result } = renderHook(() =>
useSpringVisibility({
visible: false,
active: false,
setActive: mockSetActive,
afterClose: mockAfterClose,
unmountedRef: mockUnmountedRef,
})
)

// onRest fires normally (no visibilitychange handler intervention)
expect(result.current.shouldCallAfterClose()).toBe(true)
// Second call should return false
expect(result.current.shouldCallAfterClose()).toBe(false)
})

it('should reset closedRef when visible becomes true', () => {
const { result, rerender } = renderHook(
({ visible }: { visible: boolean }) =>
useSpringVisibility({
visible,
active: true,
setActive: mockSetActive,
afterClose: mockAfterClose,
unmountedRef: mockUnmountedRef,
}),
{ initialProps: { visible: false } }
)

// Simulate afterClose being called (via onRest or visibilitychange)
// shouldCallAfterClose returns true on first call, then false on subsequent calls
expect(result.current.shouldCallAfterClose()).toBe(true)
expect(result.current.shouldCallAfterClose()).toBe(false)

// Now visible becomes true (new show cycle) - closedRef should be reset
rerender({ visible: true })

// shouldCallAfterClose should be reset and return true again
expect(result.current.shouldCallAfterClose()).toBe(true)
})

it('should not trigger on visibilitychange when document is hidden', () => {
renderHook(() =>
useSpringVisibility({
visible: false,
active: true,
setActive: mockSetActive,
afterClose: mockAfterClose,
unmountedRef: mockUnmountedRef,
})
)

Object.defineProperty(document, 'visibilityState', {
value: 'hidden',
writable: true,
})
act(() => {
document.dispatchEvent(new Event('visibilitychange'))
})

expect(mockSetActive).not.toHaveBeenCalled()
expect(mockAfterClose).not.toHaveBeenCalled()
})
})
52 changes: 52 additions & 0 deletions src/utils/use-spring-visibility.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { useIsomorphicLayoutEffect } from 'ahooks'
import type { RefObject } from 'react'
import { useEffect, useRef } from 'react'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

建议导入 useCallback 以便对 Hook 返回的函数进行记忆化处理,保持引用稳定。

Suggested change
import { useEffect, useRef } from 'react'
import { useCallback, useEffect, useRef } from 'react'


export function useSpringVisibility({
visible,
active,
setActive,
afterClose,
unmountedRef,
}: {
visible: boolean
active: boolean
setActive: (value: boolean) => void
afterClose?: () => void
unmountedRef: RefObject<boolean>
}) {
const closedRef = useRef(false)
const afterCloseRef = useRef(afterClose)
afterCloseRef.current = afterClose
const activeRef = useRef(active)
activeRef.current = active

// Reset closedRef when a new show cycle begins
useIsomorphicLayoutEffect(() => {
if (visible) {
closedRef.current = false
}
}, [visible])

useEffect(() => {
const handler = () => {
if (document.visibilityState !== 'visible') return
if (unmountedRef.current) return
if (!visible && activeRef.current && !closedRef.current) {
closedRef.current = true
setActive(false)
afterCloseRef.current?.()
}
}
document.addEventListener('visibilitychange', handler)
return () => document.removeEventListener('visibilitychange', handler)
}, [visible, setActive, unmountedRef])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

建议使用 useRef 来追踪 visible 的最新值,并将 visibleuseEffect 的依赖数组中移除。这样可以避免在每次 visible 状态改变时都重新绑定和解绑 visibilitychange 事件监听器。对于全局事件监听器,保持监听函数的稳定是更好的实践,尤其是在组件频繁开关的情况下。

  const visibleRef = useRef(visible)
  visibleRef.current = visible

  useEffect(() => {
    const handler = () => {
      if (document.visibilityState !== 'visible') return
      if (unmountedRef.current) return
      if (!visibleRef.current && activeRef.current && !closedRef.current) {
        closedRef.current = true
        setActive(false)
        afterCloseRef.current?.()
      }
    }
    document.addEventListener('visibilitychange', handler)
    return () => document.removeEventListener('visibilitychange', handler)
  }, [setActive, unmountedRef])

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

function shouldCallAfterClose(): boolean {
if (closedRef.current) return false
closedRef.current = true
return true
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

建议使用 useCallback 包裹 shouldCallAfterClose 函数。虽然目前组件中的 onRest 也是每次渲染重新创建的,但保持 Hook 返回的函数引用稳定有助于避免在 useSpring 内部产生不必要的更新逻辑。

Suggested change
function shouldCallAfterClose(): boolean {
if (closedRef.current) return false
closedRef.current = true
return true
}
const shouldCallAfterClose = useCallback((): boolean => {
if (closedRef.current) return false
closedRef.current = true
return true
}, [])


return { shouldCallAfterClose }
}
Loading