Skip to content

Commit a0b3255

Browse files
committed
Fix concurrence issues
1 parent 5b10df8 commit a0b3255

3 files changed

Lines changed: 40 additions & 23 deletions

File tree

src/main.ts

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ interface DragState {
3333

3434
type MouseHandler = (event: Event, mouse: Electron.MouseInputEvent) => void;
3535

36-
export type DraggableWindow = BaseWindow & { __wdrag__?: Draggable };
36+
type DraggableWindow = BaseWindow & { __wdrag__?: Draggable };
3737

3838
export class Draggable {
3939
private static readonly TRUE_PROMISE = Promise.resolve(true);
@@ -108,20 +108,22 @@ export class Draggable {
108108
}
109109

110110
/** Register a WebContents as a drag source for this window. */
111-
public attach(webContents: WebContents, overrideOptions?: DragOptions): void {
111+
public attach(webContents: WebContents, overrideOptions?: DragOptions): this {
112112
if (webContents.isDestroyed()) {
113113
throw new Error('Cannot attach to destroyed WebContents');
114114
}
115115

116116
if (this.optionsByWebContents.has(webContents)) {
117117
overrideOptions && this.updateOptions(webContents, overrideOptions);
118-
return;
118+
return this;
119119
}
120120

121121
let options: InternalDragOptions;
122122
if (overrideOptions) {
123-
Draggable.normalizeOptions(overrideOptions);
124-
options = { ...this.options, ...overrideOptions };
123+
// Fix: Avoid changing the user-provided options object
124+
const newOptions = { ...overrideOptions };
125+
Draggable.normalizeOptions(newOptions);
126+
options = { ...this.options, ...newOptions };
125127
} else {
126128
options = { ...this.options };
127129
}
@@ -132,34 +134,40 @@ export class Draggable {
132134
this.optionsByWebContents.set(webContents, options);
133135
webContents.on('before-mouse-event', options.eventHandler);
134136
webContents.once('destroyed', options.destroyListener);
137+
return this;
135138
}
136139

137140
/** Unregister a WebContents from dragging. */
138-
public detach(webContents: WebContents): void {
141+
public detach(webContents: WebContents): this {
139142
const options = this.optionsByWebContents.get(webContents);
140143
if (options) {
141144
webContents.removeListener('before-mouse-event', options.eventHandler!);
142145
webContents.removeListener('destroyed', options.destroyListener!);
143146
this.optionsByWebContents.delete(webContents);
144147
}
148+
return this;
145149
}
146150

147151
/** Unregister all WebContents from dragging. */
148-
public detachAll(): void {
152+
public detachAll(): this {
149153
for (const wc of this.optionsByWebContents.keys()) {
150154
this.detach(wc);
151155
}
156+
return this;
152157
}
153158

154159
/**
155160
* Disable drag for all registered WebContents and release the window reference.
156161
* @remarks After calling this method, the instance is dead and must not be reused.
157162
*/
158-
public disable(): void {
159-
if (!this.window) { return; }
163+
public disable(): this {
164+
if (!this.window) { return this; }
160165
this.detachAll();
161-
this.window.__wdrag__ = undefined;
166+
if (this.window.__wdrag__ === this) {
167+
this.window.__wdrag__ = undefined;
168+
}
162169
this.window = undefined;
170+
return this;
163171
}
164172

165173
/**
@@ -169,7 +177,7 @@ export class Draggable {
169177
* @example
170178
* drag.updateOptions({ fps: 120 });
171179
*/
172-
public updateOptions(update: Partial<DragOptions>): void;
180+
public updateOptions(update: Partial<DragOptions>): this;
173181

174182
/**
175183
* Update drag options for specific WebContents.
@@ -179,9 +187,9 @@ export class Draggable {
179187
* @example
180188
* drag.updateOptions(webContents, { fps: 120 });
181189
*/
182-
public updateOptions(wc: WebContents, update: Partial<DragOptions>): void;
190+
public updateOptions(wc: WebContents, update: Partial<DragOptions>): this;
183191

184-
public updateOptions(wcOrUpdate: WebContents | Partial<DragOptions>, update?: Partial<DragOptions>): void {
192+
public updateOptions(wcOrUpdate: WebContents | Partial<DragOptions>, update?: Partial<DragOptions>): this {
185193
const newOptions = update ?? wcOrUpdate as Partial<DragOptions>;
186194
Draggable.normalizeOptions(newOptions);
187195

@@ -193,14 +201,18 @@ export class Draggable {
193201
Object.assign(options, newOptions);
194202
}
195203
}
204+
return this;
196205
}
197206

198207
/** Retarget the instance to a new window. */
199-
public setWindow(newWin: DraggableWindow): void {
200-
this.window!.__wdrag__ = undefined; // Clear old reference
201-
202-
this.window = newWin;
203-
this.window.__wdrag__ = this; // Set new reference
208+
public setWindow(newWindow: DraggableWindow): this {
209+
const oldWin = this.window!;
210+
this.window = newWindow;
211+
if (oldWin.__wdrag__ === this) {
212+
oldWin.__wdrag__ = undefined; // Clear old reference
213+
this.window.__wdrag__ = this; // Set new reference
214+
}
215+
return this;
204216
}
205217

206218
private createEventHandler(wc: WebContents): MouseHandler {

test/sample.html

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@
44
<title>Sample</title>
55
</head>
66
<body style="margin: 0; padding: 0; cursor: default !important;">
7-
<div style="height: 100px; background-color: yellow; display: flex; align-items: center; justify-content: center; gap: 10px;">
7+
<div style="height: 100px; background-color: rgb(89, 202, 89); display: flex; align-items: center; justify-content: center; gap: 10px;">
88
<h4>Draggable!</h4>
9-
<div class="not-draggable" style="background-color: red; height: 30px; display: flex; align-items: center; justify-content: center; padding: 5px;">
9+
<div class="not-drag-1" style="background-color: rgb(194, 57, 57); display: flex; align-items: center; justify-content: center; padding: 5px;">
1010
<span>Not Draggable!</span>
1111
</div>
12-
<button>Not Draggable</button>
12+
<button>Not Draggable Button</button>
13+
<div class="not-drag-1 drag-2" style="background-color: rgb(238, 238, 29);display: flex; align-items: center; justify-content: center; padding: 5px;">10 FPS drag area</div>
1314
</div>
1415

1516
<br>
@@ -24,6 +25,5 @@ <h4>Draggable!</h4>
2425

2526
Aliquam ut pellentesque tellus, quis vulputate nisl. Phasellus vitae blandit nunc, eu sodales velit. Duis enim tellus, faucibus id arcu vitae, consectetur tempus mauris. Praesent commodo commodo dolor non malesuada. Donec sed dolor eget arcu tincidunt efficitur sit amet vel nisi. Sed consectetur tincidunt molestie. Nam at magna a odio rhoncus convallis id eu nunc. Nam luctus ut leo et viverra. Proin tempor libero vitae arcu laoreet, sed pharetra sapien rutrum. Nam nunc orci, aliquet sit amet dignissim nec, aliquet quis quam. Nulla facilisi.
2627
</span>
27-
<script> const $remote = require('electron-findbar/remote'); const $ipc = require('electron').ipcRenderer; </script>
2828
</body>
2929
</html>

test/sample.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,12 @@ function setupWindow() {
2020
window.contentView.addChildView(view);
2121
view.setBounds({ x: 0, y: 0, width: 800, height: 600 });
2222
view.webContents.loadFile(`${__dirname}/sample.html`);
23-
Draggable.from(window, { actionArea: 100 }).attach(view.webContents, { exclude: '.not-draggable, pre, button'});
23+
const dragHandler = Draggable.from(window, { actionArea: 100 }).attach(view.webContents, { exclude: '.not-drag-1, button'});
24+
Draggable.create(window, { selector: '.drag-2', fps: 10 }).attach(view.webContents);
25+
if (dragHandler !== Draggable.from(window)) {
26+
window.destroy();
27+
throw new Error('attach did not return the Draggable instance');
28+
}
2429
return window;
2530
}
2631

0 commit comments

Comments
 (0)