Skip to content

Conversation

Ritinpaul
Copy link

Fixes #2662

Overview

This PR fixes a type-safety issue in useFullscreen and adds a minimal test to ensure basic functionality.

Description

useFullscreen currently expects RefObject<HTMLElement> which is not compatible with React's useRef/createRef that can be null. This PR changes the type to RefObject<HTMLElement | null>. Also added a small cast in the catch block for better TypeScript safety.

Type of change

  • src/useFullscreen.ts:
    • Type change for ref parameter
    • Cast caught error to Error (onClose(error as Error))
  • tests/useFullscreen.test.ts:
    • Minimal test to ensure the hook is defined and returns a boolean

Checklist

  • Read the Contributing Guide
  • Perform a code self-review
  • Comment the code, particularly in hard-to-understand areas
  • Add documentation
  • Add hook's story at Storybook
  • Cover changes with tests
  • Ensure the test suite passes (yarn test)
  • Provide 100% tests coverage
  • Make sure code lints (yarn lint). Fix it with yarn lint:fix in case of failure.
  • Make sure types are fine (yarn lint:types).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useFullscreen doesn't accept a nullable ref as first parameter
1 participant