Skip to content

mark 'force' as optional property of PressEvent object#29006

Closed
Simek wants to merge 4 commits into
facebook:masterfrom
Simek:patch-3
Closed

mark 'force' as optional property of PressEvent object#29006
Simek wants to merge 4 commits into
facebook:masterfrom
Simek:patch-3

Conversation

@Simek
Copy link
Copy Markdown
Collaborator

@Simek Simek commented May 29, 2020

Summary

Refs:

According to the current implementation force will be returned only on iOS platform, only on the devices which have a hardware 3D touch capabilities.

Changelog

[General] [Fixed] - mark force as an optional property of the PressEvent object

Test Plan

Go CI!

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 29, 2020
@analysis-bot
Copy link
Copy Markdown

analysis-bot commented May 29, 2020

Platform Engine Arch Size (bytes) Diff
ios - universal 10,721,696 9,912,128

Base commit: 92160f3

@analysis-bot
Copy link
Copy Markdown

analysis-bot commented May 29, 2020

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 6,764,616 22
android hermes armeabi-v7a 6,426,372 22
android hermes x86 7,149,370 24
android hermes x86_64 7,039,909 17
android jsc arm64-v8a 8,933,111 34
android jsc armeabi-v7a 8,587,252 42
android jsc x86 8,760,966 22
android jsc x86_64 9,337,150 11

Base commit: 92160f3

@elicwhite
Copy link
Copy Markdown
Member

Thanks for working on this. Can you verify at runtime that this key doesn't exist on Android? The code you linked to isn't really definitive. For example, I could imagine Android is currently providing the force key, but it always being 1.

I'd recommend getting the event and console logging the keys on iOS showing that it includes force, and then running the same code on Android and showing that it does not include the key. That's what I'd expect to see in your test plan.

@Simek
Copy link
Copy Markdown
Collaborator Author

Simek commented May 29, 2020

@TheSavior As I were writing earlier this key is not even present on iOS, on the devices without hardware 3D Touch support, so I don't think that screenshots from Android are necessary.

This information was validated earlier, in more extensive tests during the work on the "PressEvent" and help on the "Pressable" React Native website pages.

I have also bring it up to your attention earlier:
facebook/react-native-website#1933 (comment)

iPhone SE 2nd (no force)

Screenshot 2020-05-29 at 23 44 46

iPhone 11 Pro (no force)

Screenshot 2020-05-29 at 23 47 01

iPhone 8 (force)

Screenshot 2020-05-29 at 23 49 16

Comment thread Libraries/Types/CoreEventTypes.js Outdated
@elicwhite
Copy link
Copy Markdown
Member

Thanks for those screenshots, that's great and makes it really clear what is happening.

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label May 30, 2020
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@TheSavior merged this pull request in ad2f98d.

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants