Skip to content

[Web] Change shouldBeCancelledByOther to respect blocksExternalGesture #3429

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.

Sign up for GitHub

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

Merged
m-bert merged 5 commits into from
Mar 17, 2025
Merged

[Web] Change shouldBeCancelledByOther to respect blocksExternalGesture #3429

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
92dee36
Fix shouldBeCancelled
m-bert Mar 3, 2025
e16b5c1
Merge branch 'main' into @mbert/web-blocks-external-gesture
m-bert Mar 3, 2025
94ea981
Merge branch 'main' into @mbert/web-blocks-external-gesture
m-bert Mar 6, 2025
1ec10dd
Merge branch 'main' into @mbert/web-blocks-external-gesture
m-bert Mar 14, 2025
567e70a
Change active state
m-bert Mar 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/web/tools/GestureHandlerOrchestrator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ export default class GestureHandlerOrchestrator {
}

if (handler.awaiting || handler.state === State.ACTIVE) {
// For now it always returns false
return handler.shouldBeCancelledByOther(otherHandler);
}

Expand Down
10 changes: 9 additions & 1 deletion src/web/tools/InteractionManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export default class InteractionManager {
}

public shouldHandlerBeCancelledBy(
_handler: IGestureHandler,
handler: IGestureHandler,
otherHandler: IGestureHandler
): boolean {
// We check constructor name instead of using `instanceof` in order do avoid circular dependencies
Expand All @@ -111,6 +111,14 @@ export default class InteractionManager {
const isActive = otherHandler.state === State.ACTIVE;
const isButton = otherHandler.isButton?.() === true;

if (
this.blocksHandlersRelations
.get(handler.handlerTag)
?.includes(otherHandler.handlerTag)
) {
return false;
}
Copy link
Member

@j-piasecki j-piasecki Mar 3, 2025

Choose a reason for hiding this comment

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

Does this allow for the situation where both of the gestures are active?

Copy link
Contributor Author

@m-bert m-bert Mar 6, 2025

Choose a reason for hiding this comment

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

Well, depends how do you understand active. If by having state set to 4 then yes, it is possible due to current Gesture Handler implementation.

But looking at the example code, NativeViewGestureHandler won't send event in active state since tryActivate for this handler will be called only once and it will be marked as awaitingHandler

Copy link
Member

@j-piasecki j-piasecki Mar 13, 2025

Choose a reason for hiding this comment

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

Is it possible to do the reverse? I.e. when a handler tries to activate, check if there's already an active one that is blocking the current one. If so, cancel/fail instead of activating.

Copy link
Contributor Author

@m-bert m-bert Mar 13, 2025

Choose a reason for hiding this comment

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

If I recall correctly there were some complications with this approach, but I'll double-check that


return isNativeHandler && isActive && !isButton;
}

Expand Down
Loading