Skip to content

Expose Pressability Hover config props in Pressable - #32405

Closed
Saadnajmi wants to merge 2 commits into
react:mainfrom
Saadnajmi:pressable-hover
Closed

Expose Pressability Hover config props in Pressable#32405
Saadnajmi wants to merge 2 commits into
react:mainfrom
Saadnajmi:pressable-hover

Conversation

@Saadnajmi

@SaadnajmiSaadnajmi commented Oct 14, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

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.

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

analysis-bot
analysis-bot suggested changes Oct 14, 2021
View reviewed changes

@analysis-botanalysis-bot left a comment

Copy link
Copy Markdown

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.

Comment threadLibraries/Components/Pressable/Pressable.js Outdated
@facebook-github-botfacebook-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

pull-bot commented Oct 14, 2021

Copy link
Copy Markdown

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

analysis-bot commented Oct 14, 2021

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
ios-universaln/a--

Base commit: 689bfed

@NickGerleman

NickGerleman commented Oct 14, 2021

Copy link
Copy Markdown
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

Saadnajmi commented Oct 14, 2021

Copy link
Copy Markdown
ContributorAuthor

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?
https://github.com/facebook/react-native/blob/b2e6483876cedc158684d8985f73fbf67cf3b87d/Libraries/Pressability/HoverState.js#L15

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

facebook-github-bot commented Oct 14, 2021

Copy link
Copy Markdown
Contributor

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

@SaadnajmiSaadnajmi mentioned this pull request Oct 14, 2021
Closed
@facebook-github-botfacebook-github-bot added the Merged This PR has been merged. label Oct 26, 2021
@facebook-github-bot

facebook-github-bot commented Oct 26, 2021

Copy link
Copy Markdown
Contributor

@sota000 merged this pull request in 1b30dd0.

This was referenced Oct 27, 2021
Merged
Closed
Closed
@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
@Saadnajmi
Expose Pressability Hover config props in Pressable (react#32405)
0818f75
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: react#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
@SaadnajmiSaadnajmi mentioned this pull request Nov 4, 2021
Merged
4 tasks
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Nov 4, 2021
@Saadnajmi
Expose Pressability Hover config props in Pressable (react#32405)
e03eac5
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: react#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
@SaadnajmiSaadnajmi mentioned this pull request Nov 5, 2021
Closed
8 tasks
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Dec 7, 2021
@Saadnajmi
Expose Pressability Hover config props in Pressable (react#32405)
63cb5cd
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: react#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
@Saadnajmi@pull
@ryanlntn@ntre@mganandrajMuhammad Hamza Zaman
Pull in upstream fixes to expose hover props on Pressable (#884)
f8e26e3
* 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 (react#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: react#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
@Saadnajmi
[0.63] Pull in upstream fixes to expose hover props on Pressable#884(…
29de1d0
…#887)
* Expose Pressability Hover config props in Pressable (react#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: react#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
@Saadnajmi
[0.64] Pull in upstream fixes to expose hover props on Pressable#884(…
b0c134a
…#923)
* Expose Pressability Hover config props in Pressable (react#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: react#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
This was referenced Jan 8, 2022
Merged
Merged
Merged
Merged
This was referenced Sep 20, 2022
Merged
Closed
facebook-github-bot pushed a commit that referenced this pull request Sep 21, 2022
@Saadnajmi@facebook-github-bot
Update pressable hover props (#34740)
6ba5fa9
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
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@analysis-botanalysis-botanalysis-bot requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

BugCLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.p: MicrosoftPartner: MicrosoftPartnerShared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Saadnajmi@pull-bot@analysis-bot@NickGerleman@facebook-github-bot@react-native-bot