Skip to content

[Web] Fix handling of enabled prop #3726

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

Open
wants to merge 6 commits into
base: next
Choose a base branch
from
Open

[Web] Fix handling of enabled prop #3726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c00cd6d
Change handling of enabled prop
m-bert Oct 1, 2025
24b57ac
Rename to setEnabled
m-bert Oct 2, 2025
ad10a30
Update type
m-bert Oct 2, 2025
27c6c2d
Merge branch 'next' into @mbert/change-enable-behavior-on-web
m-bert Oct 3, 2025
1f81818
Do not call onEnabledChange every time
m-bert Oct 6, 2025
5e976fa
Merge branch 'next' into @mbert/change-enable-behavior-on-web
m-bert Oct 8, 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
24 changes: 18 additions & 6 deletions packages/react-native-gesture-handler/src/web/handlers/GestureHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export default abstract class GestureHandler implements IGestureHandler {
private _state: State = State.UNDETERMINED;

private _shouldCancelWhenOutside = false;
private _enabled = false;
private _enabled: boolean | null = null;

private viewRef: number | null = null;
private propsRef: React.RefObject<PropsRef> | null = null;
Expand Down Expand Up @@ -712,16 +712,28 @@ export default abstract class GestureHandler implements IGestureHandler {
// Handling config
//

// Helper function to correctly set enabled property
private updateEnabled(enabled: boolean | undefined) {
if (enabled === undefined) {
if (this._enabled) {
return;
}

this._enabled = true;
this.delegate.onEnabledChange();
} else if (this._enabled !== enabled) {
this._enabled = enabled;
this.delegate.onEnabledChange();
}
}

public setGestureConfig(config: Config) {
this.resetConfig();
Copy link
Member

Choose a reason for hiding this comment

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

If I understand correctly, the issue is that we reset the config here, but if the gesture was disabled before, we don't reattach the event listeners. Shouldn't we also try to attach listeners when resetting the config? At first glance, it looks like it could simplify this a lot (especially if it were to be done in a setter).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shouldn't we also try to attach listeners when resetting the config?

But in that case if we have config that has enabled set to false we will first attach and then immediately detach listeners. Is that something that we want?

Copy link
Member

Choose a reason for hiding this comment

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

Can we reverse that and start with detached listeners which are then attached when config has enabled !== false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So instead of attaching listeners in resetConfig, we should detach them, and then attach if enabled !== false?

Copy link
Contributor Author

@m-bert m-bert Oct 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If I understand correctly, the issue is that we reset the config here, but if the gesture was disabled before, we don't reattach the event listeners.

Also, I don't know whether we will have any other logic that depends on enabled change, but I think that we should say that what the issue really is is that in current setup it is impossible to detect that enabled has changed to true*. This may (or may not) cause other troubles in the future.

* If we go through resetConfig first, with SharedValue it should be fine.

this.updateGestureConfig(config);
}

public updateGestureConfig(config: Config): void {
if (config.enabled !== undefined && this.enabled !== config.enabled) {
this._enabled = config.enabled;
this.delegate.onEnabledChange();
}
this.updateEnabled(config.enabled);

if (config.hitSlop !== undefined) {
this.hitSlop = config.hitSlop;
Expand Down Expand Up @@ -911,7 +923,7 @@ export default abstract class GestureHandler implements IGestureHandler {
}

protected resetConfig(): void {
this._enabled = true;
this._enabled = null;
this.manualActivation = false;
this.shouldCancelWhenOutside = false;
this.mouseButton = undefined;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-native-gesture-handler/src/web/handlers/IGestureHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export default interface IGestureHandler {
state: State;
shouldCancelWhenOutside: boolean;
shouldResetProgress: boolean;
readonly enabled: boolean;
readonly enabled: boolean | null;
readonly pointerType: PointerType;
enableContextMenu: boolean;
readonly activeCursor?: ActiveCursor;
Expand Down
9 changes: 9 additions & 0 deletions packages/react-native-gesture-handler/src/web/tools/EventManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,15 @@ export default abstract class EventManager<T> {
this.pointersInBounds.splice(index, 1);
}

public setEnabled(value: boolean | null) {
if (value) {
this.registerListeners();
} else {
this.resetManager();
this.unregisterListeners();
}
}

public resetManager(): void {
// Reseting activePointersCounter is necessary to make gestures such as pinch work properly
// There are gestures that end when there is still one active pointer (like pinch/rotation)
Expand Down
23 changes: 7 additions & 16 deletions packages/react-native-gesture-handler/src/web/tools/GestureHandlerWebDelegate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ export class GestureHandlerWebDelegate
);
}

this.isInitialized = true;

this.gestureHandler = handler;
this.view = findNodeHandle(viewRef) as unknown as HTMLElement;

Expand All @@ -48,17 +46,15 @@ export class GestureHandlerWebDelegate
touchAction: this.view.style.touchAction,
};

this.setUserSelect();
this.setTouchAction();
this.setContextMenu();

this.eventManagers.push(new PointerEventManager(this.view));
this.eventManagers.push(new KeyboardEventManager(this.view));
this.eventManagers.push(new WheelEventManager(this.view));

this.eventManagers.forEach((manager) =>
this.gestureHandler.attachEventManager(manager)
);

this.isInitialized = true;
}

detach(): void {
Expand All @@ -68,8 +64,9 @@ export class GestureHandlerWebDelegate
};

this.eventManagers.forEach((manager) => {
manager.unregisterListeners();
manager.setEnabled(false);
});

this.removeContextMenuListeners();
this._view = null;
this.eventManagers = [];
Expand Down Expand Up @@ -198,15 +195,9 @@ export class GestureHandlerWebDelegate
this.setTouchAction();
this.setContextMenu();

if (this.gestureHandler.enabled) {
this.eventManagers.forEach((manager) => {
manager.registerListeners();
});
} else {
this.eventManagers.forEach((manager) => {
manager.unregisterListeners();
});
}
this.eventManagers.forEach((manager) => {
manager.setEnabled(this.gestureHandler.enabled);
});
}

onBegin(): void {
Expand Down
Loading