-
Notifications
You must be signed in to change notification settings - Fork 163
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
Switch Tabs components to use React Native Pressable #2145
Conversation
@@ -146,7 +146,7 @@ export interface TabsItemInfo { | |||
} | |||
|
|||
export interface TabsItemSlotProps { | |||
root: React.PropsWithRef<IViewProps>; | |||
root: React.PropsWithRef<PressableProps>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically we lose the adapter props here. Will have to think about that. Could probably easily make an "IPressableProps" if I really wanted to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah in practice as long as the props we expose for the component stay constant I think we're ok... Even if props that don't match the type are passed in, they're still on the object so they should get passed through, they just will make TS angry if someone tries to modify the root slot directly and add props that don't exist on PressableProps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So really it'll be more of a problem for us than for clients.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think type safety is still a but of an open problem in FURN. Adapters is a step towards it but if we enable platforms specific type safety checking via rnx-kit, there's still issues. I'll try to spend a day thinking about how best to not regress the experience for ourselves.
I can test it for win32/windows for you. |
Today is "set up computers day" so I should be able to do it, but if it's easy enough after I change the slot, I appreciate it! |
Platforms Impacted
Description of changes
Following up from #1070, let's move over components off of
useAsPressable
/ our fork of Pressable and onto Pressable from React Native, which contains the newer support for pointer events / android ripple / codegen commands.Verification
Hover, Focus, and Press work as expected on macOS. E2E testing on CI should confirm as much for windows/win32, but I'll try and test it out myself too.
Screen.Recording.2022-09-21.at.2.33.39.AM.mov
Pull request checklist
This PR has considered (when applicable):