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

Add a new gesture relation - blocksHandlers - working like reversed waitFor #2627

Merged
merged 16 commits into from
Nov 21, 2023

Conversation

j-piasecki
Copy link
Member

@j-piasecki j-piasecki commented Oct 10, 2023

Description

Introduces a new relation between gestures, which allows to set one gesture recognizer as blocking for another one. It's basically the same as waitFor but in the other direction - waitFor allows to specify which gestures must fail before this one can activate. The new prop allows to specify which gestures should require this one to fail before they can activate.

For the name, I think we've settled at blocksHandlers for the old API and .blocksExternalGesture() for the new API.

There's no need to add another type of composition since it accomplishes the same thing as Gesture.Exclusive, and is inherently a between-components relation.

Test plan

Tested on the Example app

Since it relies on already existing logic, I assume it will work correctly. I would like to do more testing nonetheless.
Also needs to be tested on physical iOS device with multi-touch gestures.

@j-piasecki j-piasecki changed the title Add a new gesture relation - shouldBeRequiredToFailBy - working like reversed waitFor Add a new gesture relation - requiredToFailBy - working like reversed waitFor Oct 10, 2023
docs/docs/fundamentals/gesture-composition.md Outdated Show resolved Hide resolved
docs/docs/fundamentals/gesture-composition.md Outdated Show resolved Hide resolved

## requiredToFailByExternalGesture

`requiredToFailByExternalGesture` works similarily to `requireExternalGestureToFail` but the direction of the relation is reversed - insted of making the gesture it's called on wait for failure of gestures passed as arguments, it's making gestures passed as arguments wait for the gesture it's called on. It's especially usefull for making lists where the `ScrollView` component needs to wait for every gesture underneath it. All that's required is passing a ref of the `ScrollView` to the gesture object, for example:
Copy link
Member

Choose a reason for hiding this comment

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

The first sentence is a complete gibberish :grief:

also, typos in insted and usefull

also, the last sentence I'd write something like:

All that's required to do is to pass a ref

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you take a look at 03878fd and 5cf17b9?


## simultaneousWithExternalGesture

`simultaneousWithExternalGesture` allows gestures across different components to be recognized simultaneously. We can modify the example from `requireExternalGestureToFail` to showcase this: let's say you have two nested views, again both with tap gesture attached. This time, both of them require one tap, but tapping the inner one should also activate the gesture attached to the outer view:
Copy link
Member

Choose a reason for hiding this comment

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

People rarely read documentation from start to finish. They mostly skim the headings and code so refering to previous sections isn't that useful

It's better to repeat the full intent of the paragraph rather than refering to previous sections

Copy link
Member

@kmagiera kmagiera left a comment

Choose a reason for hiding this comment

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

Code changes look good. I wonder why is this marked as draft? Also would be good to explain the plans for adding this to web.

The only comment I have is around the naming. I understand that this aims to replicate the naming from the UIKit gesture recognizer API, but I find it very confusing. I'd consider going with some different naming convention not revolving around "waiting". Some ideas:
1)blocksRecognizer={ref} – this name would suggest that recognizer ref will be blocked by the current one, which IMO illustrates this relation pretty well
2) preempts={ref} – this will suggest that this recognizer will take precedence over ref

@j-piasecki
Copy link
Member Author

Code changes look good. I wonder why is this marked as draft? Also would be good to explain the plans for adding this to web.

It's marked as a draft because it's not finished yet, as you noticed anything related to web is missing.

As for the name, blocksRecognizer sounds like the best option to me.

@j-piasecki j-piasecki changed the title Add a new gesture relation - requiredToFailBy - working like reversed waitFor Add a new gesture relation - blocksRecognizers - working like reversed waitFor Oct 31, 2023
@j-piasecki j-piasecki marked this pull request as ready for review October 31, 2023 10:35
Copy link
Member

@kmagiera kmagiera left a comment

Choose a reason for hiding this comment

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

Accepting this once again as the code looks good. Just wanted to note that my prev suggestion about the name might not be in line with the naming we use here, specifically, we don't use word "recognizers" elsewhere AFAIK and it should rather be blocksHandlers, similarily to "simultaniousHandlers", what do you think?

@j-piasecki
Copy link
Member Author

Just wanted to note that my prev suggestion about the name might not be in line with the naming we use here, specifically, we don't use word "recognizers" elsewhere AFAIK and it should rather be blocksHandlers, similarily to "simultaniousHandlers", what do you think?

"recognizers" is used in some places but only internally. In public APIs, we use "handlers", so the change makes perfect sense.

@j-piasecki j-piasecki changed the title Add a new gesture relation - blocksRecognizers - working like reversed waitFor Add a new gesture relation - blocksHandlers - working like reversed waitFor Oct 31, 2023
Copy link
Member

@kacperkapusciak kacperkapusciak left a comment

Choose a reason for hiding this comment

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

:shipit:

@j-piasecki j-piasecki merged commit 510905e into main Nov 21, 2023
7 checks passed
@j-piasecki j-piasecki deleted the @jpiasecki/roFtiaw branch November 21, 2023 09:14
renovate bot referenced this pull request in valora-inc/wallet Jan 7, 2024
…4711)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[react-native-gesture-handler](https://togithub.com/software-mansion/react-native-gesture-handler)
| [`^2.13.4` ->
`^2.14.0`](https://renovatebot.com/diffs/npm/react-native-gesture-handler/2.13.4/2.14.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/react-native-gesture-handler/2.14.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/react-native-gesture-handler/2.14.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/react-native-gesture-handler/2.13.4/2.14.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/react-native-gesture-handler/2.13.4/2.14.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>software-mansion/react-native-gesture-handler
(react-native-gesture-handler)</summary>

###
[`v2.14.0`](https://togithub.com/software-mansion/react-native-gesture-handler/releases/tag/2.14.0)

[Compare
Source](https://togithub.com/software-mansion/react-native-gesture-handler/compare/2.13.4...2.14.0)

#### ❗ Important changes

- Remove Gesture Handler limits on Android by
[@&#8203;m-bert](https://togithub.com/m-bert) in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2672](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2672)
- Add a new gesture relation - `blocksHandlers` - working like `reversed
waitFor` by [@&#8203;j-piasecki](https://togithub.com/j-piasecki) in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2627](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2627)
- Support React Native 0.73 by
[@&#8203;j-piasecki](https://togithub.com/j-piasecki) in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2671](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2671)

#### 🐛 Bug fixes

- Fix double `onPress` by [@&#8203;m-bert](https://togithub.com/m-bert)
in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2657](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2657)
- Fix tvOS build by [@&#8203;m-bert](https://togithub.com/m-bert) in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2654](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2654)
- Add a non-null assertion for
`reactApplicationContext.javaScriptContextHolder` by
[@&#8203;UNIDY2002](https://togithub.com/UNIDY2002) in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2662](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2662)
- Move REACT_NATIVE_VERSION to native-only code by
[@&#8203;chriscoomber](https://togithub.com/chriscoomber) in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2666](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2666)
- Fix selection when TalkBack is enabled by
[@&#8203;m-bert](https://togithub.com/m-bert) in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2669](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2669)
- Fix inputs focus on safari by
[@&#8203;m-bert](https://togithub.com/m-bert) in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2675](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2675)

#### 🔢 Miscellaneous

- Bump browserify-sign from 4.2.1 to 4.2.2 in /example by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2661](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2661)
- Change Alert to console.log in new docs by
[@&#8203;m-bert](https://togithub.com/m-bert) in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2670](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2670)
- Remove `handlersToCancel` on web. by
[@&#8203;m-bert](https://togithub.com/m-bert) in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2679](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2679)

#### New Contributors

- [@&#8203;chriscoomber](https://togithub.com/chriscoomber) made their
first contribution in
[https://github.com/software-mansion/react-native-gesture-handler/pull/2666](https://togithub.com/software-mansion/react-native-gesture-handler/pull/2666)

**Full Changelog**:
software-mansion/react-native-gesture-handler@2.13.4...2.14.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 5pm,every weekend" in timezone
America/Los_Angeles, Automerge - "after 5pm,every weekend" in timezone
America/Los_Angeles.

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/valora-inc/wallet).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMjEuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEyMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: valora-bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants