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

Expose Pressability Hover config props in Pressable #32405

Closed
wants to merge 2 commits into from

Conversation

Saadnajmi
Copy link
Contributor

@Saadnajmi Saadnajmi commented Oct 14, 2021

Summary

Several desktop forks (react-native-macos, react-native-windows, react-native-web) support mouse events, and while the stock Pressable component has the ability to support mouse events, it seems we aren't forwarding some props properly from Pressable -> Pressability.

Pressability will calculate onMouseEnter / onMouseLeave event handlers based on the onHoverIn/onHoverOut callbacks passed into PressabilityConfig.

const mouseEventHandlers =

However, Pressable does not pass take in onHoverIn/onHoverOut props to pass to PressabilityConfig, so we can't take advantage of this functionality. This change should simply address that by passing the props through.

Changelog

[General] [Fixed] - Pressable not passing hover props and event handlers to PressabilityConfig

Test Plan

I fixed a similar issue in react-native-macos that I am now trying to contribute back upstream. microsoft#855

Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

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

Code analysis results:

  • eslint found some issues. Run yarn lint --fix to automatically fix problems.

Libraries/Components/Pressable/Pressable.js Outdated Show resolved Hide resolved
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Oct 14, 2021
@pull-bot
Copy link

PR build artifact for f516aab is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 689bfed

@NickGerleman
Copy link
Contributor

I think Pressability will test against running on react-native-web before processing onMouseEnter, etc for hover. Would make sense to make that generic if exposing I think.

@Saadnajmi
Copy link
Contributor Author

I think Pressability will test against running on react-native-web before processing onMouseEnter, etc for hover. Would make sense to make that generic if exposing I think.

Do you basically mean, removing this check?

if (Platform.OS === 'web') {

Note for others: we override this check in react-native-macos to return true if the platform is macOS. I'm sure other platforms do something similar

@facebook-github-bot
Copy link
Contributor

@sota000 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@sota000 merged this pull request in 1b30dd0.

@Saadnajmi Saadnajmi deleted the pressable-hover branch November 4, 2021 19:17
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Nov 4, 2021
Summary:
Several desktop forks (`react-native-macos`, `react-native-windows`, `react-native-web`) support mouse events, and while the stock Pressable component has the ability to support mouse events, it seems we aren't forwarding some props properly from Pressable -> Pressability.

Pressability will calculate onMouseEnter / onMouseLeave event handlers based on the `onHoverIn/onHoverOut` callbacks passed into PressabilityConfig.
https://github.com/facebook/react-native/blob/ad0d4534a751ed05f84ff971714c8f7a4d1deb3a/Libraries/Pressability/Pressability.js#L552
 However, Pressable does not pass take in onHoverIn/onHoverOut props to pass to PressabilityConfig, so we can't take advantage of this functionality. This change should simply address that by passing the props through.

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[General] [Fixed] - Pressabel not passing hover props and event handlers to PressabilityConfig

Pull Request resolved: facebook#32405

Test Plan: I fixed a similar issue in `react-native-macos` that I am now trying to contribute back upstream. microsoft#855

Reviewed By: yungsters

Differential Revision: D31667737

Pulled By: sota000

fbshipit-source-id: f0bbe48302703bb2c45280d2afeec8d7a4586b6a
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Nov 4, 2021
Summary:
Several desktop forks (`react-native-macos`, `react-native-windows`, `react-native-web`) support mouse events, and while the stock Pressable component has the ability to support mouse events, it seems we aren't forwarding some props properly from Pressable -> Pressability.

Pressability will calculate onMouseEnter / onMouseLeave event handlers based on the `onHoverIn/onHoverOut` callbacks passed into PressabilityConfig.
https://github.com/facebook/react-native/blob/ad0d4534a751ed05f84ff971714c8f7a4d1deb3a/Libraries/Pressability/Pressability.js#L552
 However, Pressable does not pass take in onHoverIn/onHoverOut props to pass to PressabilityConfig, so we can't take advantage of this functionality. This change should simply address that by passing the props through.

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[General] [Fixed] - Pressabel not passing hover props and event handlers to PressabilityConfig

Pull Request resolved: facebook#32405

Test Plan: I fixed a similar issue in `react-native-macos` that I am now trying to contribute back upstream. microsoft#855

Reviewed By: yungsters

Differential Revision: D31667737

Pulled By: sota000

fbshipit-source-id: f0bbe48302703bb2c45280d2afeec8d7a4586b6a
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Dec 7, 2021
Summary:
Several desktop forks (`react-native-macos`, `react-native-windows`, `react-native-web`) support mouse events, and while the stock Pressable component has the ability to support mouse events, it seems we aren't forwarding some props properly from Pressable -> Pressability.

Pressability will calculate onMouseEnter / onMouseLeave event handlers based on the `onHoverIn/onHoverOut` callbacks passed into PressabilityConfig.
https://github.com/facebook/react-native/blob/ad0d4534a751ed05f84ff971714c8f7a4d1deb3a/Libraries/Pressability/Pressability.js#L552
 However, Pressable does not pass take in onHoverIn/onHoverOut props to pass to PressabilityConfig, so we can't take advantage of this functionality. This change should simply address that by passing the props through.

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[General] [Fixed] - Pressabel not passing hover props and event handlers to PressabilityConfig

Pull Request resolved: facebook#32405

Test Plan: I fixed a similar issue in `react-native-macos` that I am now trying to contribute back upstream. microsoft#855

Reviewed By: yungsters

Differential Revision: D31667737

Pulled By: sota000

fbshipit-source-id: f0bbe48302703bb2c45280d2afeec8d7a4586b6a
harrieshin pushed a commit to microsoft/react-native-macos that referenced this pull request Dec 7, 2021
* add pull yml

* match handleOpenURLNotification event payload with iOS (#755) (#2)

Co-authored-by: Ryan Linton <ryanlntn@gmail.com>

* [pull] master from microsoft:master (#11)

* Deprecated api (#853)

* Remove deprecated/unused context param
* Update a few Mac deprecated APIs

* Packing RN dependencies, hermes and ignoring javadoc failure,  (#852)

* Ignore javadoc failure

* Bringing few more changes from 0.63-stable

* Fixing a patch in engine selection

* Fixing a patch in nuget spec

* Fixing the output directory of nuget pack

* Packaging dependencies in the nuget

* Fix onMouseEnter/onMouseLeave callbacks not firing on Pressable (#855)

* add pull yml

* match handleOpenURLNotification event payload with iOS (#755) (#2)

Co-authored-by: Ryan Linton <ryanlntn@gmail.com>

* fix mouse evetns on pressable

* delete extra yml from this branch

* Add macOS tags

* reorder props to have onMouseEnter/onMouseLeave always be before onPress

Co-authored-by: pull[bot] <39814207+pull[bot]@users.noreply.github.com>
Co-authored-by: Ryan Linton <ryanlntn@gmail.com>

* Grammar fixes. (#856)

Updates simple grammar issues.

Co-authored-by: Nick Trescases <42704557+ntre@users.noreply.github.com>
Co-authored-by: Anandraj <anandrag@microsoft.com>
Co-authored-by: Saad Najmi <saadnajmi2@gmail.com>
Co-authored-by: pull[bot] <39814207+pull[bot]@users.noreply.github.com>
Co-authored-by: Ryan Linton <ryanlntn@gmail.com>
Co-authored-by: Muhammad Hamza Zaman <mh.zaman.4069@gmail.com>

* Expose Pressability Hover config props in Pressable (facebook#32405)

Summary:
Several desktop forks (`react-native-macos`, `react-native-windows`, `react-native-web`) support mouse events, and while the stock Pressable component has the ability to support mouse events, it seems we aren't forwarding some props properly from Pressable -> Pressability.

Pressability will calculate onMouseEnter / onMouseLeave event handlers based on the `onHoverIn/onHoverOut` callbacks passed into PressabilityConfig.
https://github.com/facebook/react-native/blob/ad0d4534a751ed05f84ff971714c8f7a4d1deb3a/Libraries/Pressability/Pressability.js#L552
 However, Pressable does not pass take in onHoverIn/onHoverOut props to pass to PressabilityConfig, so we can't take advantage of this functionality. This change should simply address that by passing the props through.

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[General] [Fixed] - Pressabel not passing hover props and event handlers to PressabilityConfig

Pull Request resolved: facebook#32405

Test Plan: I fixed a similar issue in `react-native-macos` that I am now trying to contribute back upstream. #855

Reviewed By: yungsters

Differential Revision: D31667737

Pulled By: sota000

fbshipit-source-id: f0bbe48302703bb2c45280d2afeec8d7a4586b6a

* Extra followup changes

* remove some extra fork differences from change

* Add back type

* update podfile

Co-authored-by: pull[bot] <39814207+pull[bot]@users.noreply.github.com>
Co-authored-by: Ryan Linton <ryanlntn@gmail.com>
Co-authored-by: Nick Trescases <42704557+ntre@users.noreply.github.com>
Co-authored-by: Anandraj <anandrag@microsoft.com>
Co-authored-by: Muhammad Hamza Zaman <mh.zaman.4069@gmail.com>
harrieshin pushed a commit to microsoft/react-native-macos that referenced this pull request Dec 7, 2021
…#887)

* Expose Pressability Hover config props in Pressable (facebook#32405)

Summary:
Several desktop forks (`react-native-macos`, `react-native-windows`, `react-native-web`) support mouse events, and while the stock Pressable component has the ability to support mouse events, it seems we aren't forwarding some props properly from Pressable -> Pressability.

Pressability will calculate onMouseEnter / onMouseLeave event handlers based on the `onHoverIn/onHoverOut` callbacks passed into PressabilityConfig.
https://github.com/facebook/react-native/blob/ad0d4534a751ed05f84ff971714c8f7a4d1deb3a/Libraries/Pressability/Pressability.js#L552
 However, Pressable does not pass take in onHoverIn/onHoverOut props to pass to PressabilityConfig, so we can't take advantage of this functionality. This change should simply address that by passing the props through.

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[General] [Fixed] - Pressabel not passing hover props and event handlers to PressabilityConfig

Pull Request resolved: facebook#32405

Test Plan: I fixed a similar issue in `react-native-macos` that I am now trying to contribute back upstream. #855

Reviewed By: yungsters

Differential Revision: D31667737

Pulled By: sota000

fbshipit-source-id: f0bbe48302703bb2c45280d2afeec8d7a4586b6a

* Additional changes

* undo extra unnecessary changes

* remove another line

* update podfile lock
harrieshin pushed a commit to microsoft/react-native-macos that referenced this pull request Dec 7, 2021
…#923)

* Expose Pressability Hover config props in Pressable (facebook#32405)

Summary:
Several desktop forks (`react-native-macos`, `react-native-windows`, `react-native-web`) support mouse events, and while the stock Pressable component has the ability to support mouse events, it seems we aren't forwarding some props properly from Pressable -> Pressability.

Pressability will calculate onMouseEnter / onMouseLeave event handlers based on the `onHoverIn/onHoverOut` callbacks passed into PressabilityConfig.
https://github.com/facebook/react-native/blob/ad0d4534a751ed05f84ff971714c8f7a4d1deb3a/Libraries/Pressability/Pressability.js#L552
 However, Pressable does not pass take in onHoverIn/onHoverOut props to pass to PressabilityConfig, so we can't take advantage of this functionality. This change should simply address that by passing the props through.

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[General] [Fixed] - Pressabel not passing hover props and event handlers to PressabilityConfig

Pull Request resolved: facebook#32405

Test Plan: I fixed a similar issue in `react-native-macos` that I am now trying to contribute back upstream. #855

Reviewed By: yungsters

Differential Revision: D31667737

Pulled By: sota000

fbshipit-source-id: f0bbe48302703bb2c45280d2afeec8d7a4586b6a

* Add back macOS test

* update podfile

* remove unnecessary line

* update podfile
facebook-github-bot pushed a commit that referenced this pull request Sep 21, 2022
Summary:
This is a mirror of DefinitelyTyped/DefinitelyTyped#62344 . I am updating some typescript types for props that were added with #32405

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[General] [Fixed] - Fixed missing Pressable hover props in typescript definition

Pull Request resolved: #34740

Test Plan: Added a test case.

Reviewed By: lunaleaps

Differential Revision: D39688606

Pulled By: cortinico

fbshipit-source-id: 98ea5f64e6ac264772466455e6d5ab99a8427d3a
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
This is a mirror of DefinitelyTyped/DefinitelyTyped#62344 . I am updating some typescript types for props that were added with facebook#32405

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[General] [Fixed] - Fixed missing Pressable hover props in typescript definition

Pull Request resolved: facebook#34740

Test Plan: Added a test case.

Reviewed By: lunaleaps

Differential Revision: D39688606

Pulled By: cortinico

fbshipit-source-id: 98ea5f64e6ac264772466455e6d5ab99a8427d3a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Microsoft Partner: Microsoft Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants