-
Notifications
You must be signed in to change notification settings - Fork 5.7k
docs(TouchableWithoutFeedback): fix onLongPress timing to 500ms #4945
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
base: main
Are you sure you want to change the base?
docs(TouchableWithoutFeedback): fix onLongPress timing to 500ms #4945
Conversation
|
Hi @Cjgfree123! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
❌ Deploy Preview for react-native failed.
|
|
Signed the CLA. Thanks!
|
|
Hey @Cjgfree123, thanks for the update! 👍 Can you provide a reference from core repo code to confirm that 500 ms is the correct value? |
// packages/react-native/Libraries/Components/Touchable/TouchableWithoutFeedback.js
const pressabilityConfig = useMemo(
() => ({
...
delayLongPress: delayLongPress,
delayPressIn: delayPressIn,
onLongPress: onLongPress,
onPress: onPress,
onPressIn: onPressIn,
}),
[...]
);
const eventHandlers = new Pressability(pressabilityConfig).getEventHandlers()
// packages/react-native/Libraries/Pressability/Pressability.js
const DEFAULT_LONG_PRESS_DELAY_MS = 500;
_createEventHandlers() {
onResponderGrant() {
/**
* Equivalent to: Math.max(0, this._config.delayPressIn ?? 0)
*
* If the caller does not pass props.delayPressIn, the computed delayPressIn
* value here will be 0.
*/
const delayPressIn = normalizeDelay(this._config.delayPressIn);
// ...
/**
* Equivalent to: Math.max(10, this._config.delayLongPress ?? 500 - delayPressIn)
*
* If the caller does not pass props.delayPressIn and props.delayLongPress,
* this effectively becomes: Math.max(10, 500) = 500ms.
*
* Therefore, by default, onLongPress fires after 500ms.
*/
const delayLongPress = normalizeDelay(
this._config.delayLongPress,
10,
DEFAULT_LONG_PRESS_DELAY_MS - delayPressIn,
);
this._longPressDelayTimeout = setTimeout(() => {
this._handleLongPress(event);
}, delayLongPress + delayPressIn);
}
}
function normalizeDelay(
delay: ?number,
min: number = 0,
fallback: number = 0,
): number {
return Math.max(min, delay ?? fallback);
} |
|
Thanks for linking the reference @Cjgfree123, however, I think that 370ms delay is correct in there since it's 500ms (DEFAULT_LONG_PRESS_DELAY_MS ) - 130ms (DEFAULT_MIN_PRESS_DURATION): We probably should explain it better, where the values comes from (how it's calculated) and mention also minimal press duration, instead of changing the value. |
Summary
Update TouchableWithoutFeedback docs to state onLongPress fires after 500ms (was 370ms).
Motivation
The 370ms value is incorrect and inconsistent with the current press system defaults.
Changes
docs/touchablewithoutfeedback.md: replace “370 milliseconds” with “500 milliseconds”.
Test Plan
Ran yarn lint / yarn prettier in ./website.
Verified copy for accuracy; no runtime changes.