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

Better types and tests for useKeyProps #2468

Merged
merged 10 commits into from
Dec 30, 2022
Merged

Conversation

Saadnajmi
Copy link
Collaborator

@Saadnajmi Saadnajmi commented Dec 28, 2022

Platforms Impacted

  • iOS
  • macOS
  • win32 (Office)
  • windows
  • android

Description of changes

This PR makes the following improvements to useKeyProps:

  1. Moves the associated types to platform specific type files. That should set us up better for when we enable Typescript's platform specific type checking
  2. Combines the useKeyProps.<platform>.ts files into one. There was starting to be a lot of duplicated logic. Now that the types are separated out, I think the main file can be one combined one.
  3. Adds some simple tests that ensure that we can use our hook without causing unnecessary extra renders
  4. Removes some uses of a type from our deprecated copy of Pressability in favor of the actual type from React Native Core / macOS / windows.

All in all, this is mostly a quality of life improvement.

Verification

CI should pass

Pull request checklist

This PR has considered (when applicable):

  • Automated Tests
  • Documentation and examples
  • Keyboard Accessibility
  • Voiceover
  • Internationalization and Right-to-left Layouts

@Saadnajmi Saadnajmi marked this pull request as ready for review December 28, 2022 23:52
@Saadnajmi Saadnajmi requested a review from a team as a code owner December 28, 2022 23:52
@Saadnajmi Saadnajmi changed the title Better types + tests for useKeyProps Better types and tests for useKeyProps Dec 28, 2022
@Saadnajmi Saadnajmi merged commit 74a8dcd into microsoft:main Dec 30, 2022
@Saadnajmi Saadnajmi mentioned this pull request Jan 3, 2023
10 tasks
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.

3 participants