Skip to content

Commit 8f7e525

Browse files
authored
Fix mobile tasks, annoying caret jumping and empty widgets on navigation (#1890)
1 parent 4db4923 commit 8f7e525

File tree

9 files changed

+164
-55
lines changed

9 files changed

+164
-55
lines changed

client/codemirror/clean.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ export function cleanModePlugins(client: Client) {
5555
// Propagate click event from checkbox
5656
void client.dispatchClickEvent(clickEvent);
5757
},
58+
getView: () => client.editorView,
5859
}),
5960
listBulletPlugin(),
6061
tablePlugin(client),

client/codemirror/editor_state.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,9 @@ export function createEditorState(
270270
}),
271271
ViewPlugin.fromClass(
272272
class {
273+
// Track file changed during an IME composition session
274+
private composingDirty = false;
275+
273276
update(update: ViewUpdate): void {
274277
if (update.transactions.length > 0) {
275278
for (const tr of update.transactions) {
@@ -290,6 +293,15 @@ export function createEditorState(
290293
) {
291294
return;
292295
}
296+
297+
// Defer save and event dispatch during IME composition
298+
if (update.view.composing) {
299+
// Mark dirty so we flush when composition ends
300+
this.composingDirty = true;
301+
client.ui.viewDispatch({ type: "page-changed" });
302+
return;
303+
}
304+
293305
const changes: TextChange[] = [];
294306
update.changes.iterChanges((fromA, toA, fromB, toB, inserted) =>
295307
changes.push({
@@ -302,6 +314,12 @@ export function createEditorState(
302314
client.ui.viewDispatch({ type: "page-changed" });
303315
client.contentManager.debouncedUpdateEvent();
304316
client.save().catch((e) => console.error("Error saving", e));
317+
this.composingDirty = false;
318+
} else if (this.composingDirty && !update.view.composing) {
319+
// Flush now because composition ended without file changes
320+
this.composingDirty = false;
321+
client.contentManager.debouncedUpdateEvent();
322+
client.save().catch((e) => console.error("Error saving", e));
305323
}
306324
}
307325
},

client/codemirror/lua_widget.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -293,10 +293,14 @@ export class LuaWidget extends WidgetType {
293293
this.opts.cacheKey,
294294
div.offsetHeight,
295295
);
296-
// Because of the rejiggering of the DOM, we need to do a no-op cursor move to make sure it's positioned correctly
297-
this.opts.client.editorView.dispatch({
298-
selection: this.opts.client.editorView.state.selection,
299-
});
296+
// Skip during IME composition to avoid caret jumps
297+
if (!this.opts.client.editorView.composing) {
298+
// Because of the rejiggering of the DOM, we need to do a no-op
299+
// cursor move to make sure it's positioned correctly
300+
this.opts.client.editorView.dispatch({
301+
selection: this.opts.client.editorView.state.selection,
302+
});
303+
}
300304
});
301305
}
302306

client/codemirror/task.ts

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
import { syntaxTree } from "@codemirror/language";
2-
import { Decoration, WidgetType } from "@codemirror/view";
2+
import { Decoration, type EditorView, WidgetType } from "@codemirror/view";
33
import type { NodeType } from "@lezer/common";
44
import { decoratorStateField, isCursorInRange } from "./util.ts";
55

66
/**
77
* Widget to render checkbox for a task list item.
88
*/
99
class CheckboxWidget extends WidgetType {
10+
private dom?: HTMLElement;
11+
1012
constructor(
1113
public checked: boolean,
12-
readonly pos: number,
14+
readonly fallbackPos: number,
1315
readonly clickCallback: (pos: number) => void,
16+
readonly getView: () => EditorView | null,
1417
) {
1518
super();
1619
}
@@ -27,17 +30,55 @@ class CheckboxWidget extends WidgetType {
2730
});
2831
checkbox.addEventListener("mouseup", (e) => {
2932
e.stopPropagation();
30-
this.clickCallback(this.pos);
33+
// Resolve the current document position at click time which
34+
// prevents stale position corruption when the document has been
35+
// edited since the decoration was created
36+
let pos = this.fallbackPos;
37+
const view = this.getView();
38+
if (view && this.dom) {
39+
try {
40+
const domPos = view.posAtDOM(this.dom, 0);
41+
pos = domPos + 1;
42+
} catch {
43+
// Use fallback
44+
}
45+
}
46+
this.clickCallback(pos);
47+
});
48+
// Touch handling for mobile
49+
let touchCount = 0;
50+
checkbox.addEventListener("touchmove", () => {
51+
touchCount++;
52+
});
53+
checkbox.addEventListener("touchend", (e) => {
54+
if (touchCount === 0) {
55+
e.stopPropagation();
56+
e.preventDefault();
57+
let pos = this.fallbackPos;
58+
const view = this.getView();
59+
if (view && this.dom) {
60+
try {
61+
pos = view.posAtDOM(this.dom, 0) + 1;
62+
} catch {
63+
// use fallback
64+
}
65+
}
66+
this.clickCallback(pos);
67+
}
68+
touchCount = 0;
3169
});
3270
wrap.appendChild(checkbox);
71+
this.dom = wrap;
3372
return wrap;
3473
}
3574
}
3675

3776
export function taskListPlugin({
3877
onCheckboxClick,
78+
getView,
3979
}: {
4080
onCheckboxClick: (pos: number) => void;
81+
getView: () => EditorView | null;
4182
}) {
4283
return decoratorStateField((state) => {
4384
const widgets: any[] = [];
@@ -78,6 +119,7 @@ export function taskListPlugin({
78119
checkboxStatus,
79120
from + nfrom + 1,
80121
onCheckboxClick,
122+
getView,
81123
),
82124
});
83125
widgets.push(dec.range(from + nfrom, from + nto));

client/codemirror/util.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,15 @@ export function decoratorStateField(
103103
},
104104

105105
update(value: DecorationSet, tr: Transaction) {
106+
// Avoid full recomputation of decorations during IME composition
107+
// but map existing decoration ranges through the changes instead.
108+
if (tr.isUserEvent("input.type.compose")) {
109+
if (tr.docChanged) {
110+
return value.map(tr.changes);
111+
}
112+
return value;
113+
}
114+
106115
if (tr.isUserEvent("select.pointer")) return value;
107116
return stateToDecoratorMapper(tr.state);
108117
},

client/content_manager.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,16 @@ export class ContentManager {
7474

7575
return resolve();
7676
} else {
77+
// Do not save while IME composition is active
78+
if (this.client.editorView.composing) {
79+
// Re-schedule save after composition likely ends
80+
this.saveTimeout = setTimeout(
81+
this.save.bind(this),
82+
autoSaveInterval,
83+
);
84+
return resolve();
85+
}
86+
7787
console.log("Saving page", this.client.currentPath());
7888
void this.client.dispatchAppEvent(
7989
"editor:pageSaving",
@@ -108,8 +118,12 @@ export class ContentManager {
108118
meta: enrichedMeta,
109119
});
110120

111-
// Trigger editor re-render to update Lua widgets with the new metadata
112-
this.client.editorView.dispatch({});
121+
// Skip during IME composition
122+
if (!this.client.editorView.composing) {
123+
// Trigger editor re-render to update Lua widgets
124+
// with the new metadata
125+
this.client.editorView.dispatch({});
126+
}
113127
}
114128
})
115129
.catch((e) => {
@@ -158,6 +172,7 @@ export class ContentManager {
158172
if (previousPath) {
159173
this.client.space.unwatchFile(previousPath);
160174
await this.save(true);
175+
await this.client.objectIndex.awaitIndexQueueDrain();
161176
}
162177

163178
// This can throw, but that will be catched and handled upstream.
@@ -225,6 +240,7 @@ export class ContentManager {
225240
if (previousPath) {
226241
this.client.space.unwatchFile(previousPath);
227242
await this.save(true);
243+
await this.client.objectIndex.awaitIndexQueueDrain();
228244
}
229245

230246
// Fetch next page to open

client/data/object_index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,10 @@ export class ObjectIndex {
190190
return this.ds.get(indexVersionKey);
191191
}
192192

193+
async awaitIndexQueueDrain(): Promise<void> {
194+
await this.mq.awaitEmptyQueue("indexQueue");
195+
}
196+
193197
async markFullIndexComplete() {
194198
await this.ds.set(indexVersionKey, desiredIndexVersion);
195199
}

plug-api/lib/async.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,25 @@
1-
export function throttle(func: () => void, limit: number): () => void {
1+
export function throttle(
2+
func: () => void,
3+
limit: number,
4+
): (() => void) & { flush(): void } {
25
let timer: any = null;
3-
return () => {
6+
const throttled = () => {
47
if (!timer) {
58
timer = setTimeout(() => {
69
func();
710
timer = null;
811
}, limit);
912
}
1013
};
14+
// Immediately execute any pending call and cancel the timer
15+
throttled.flush = () => {
16+
if (timer) {
17+
clearTimeout(timer);
18+
timer = null;
19+
func();
20+
}
21+
};
22+
return throttled;
1123
}
1224

1325
export function throttleImmediately(

plugs/index/task.ts

Lines changed: 47 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ export async function updateTaskState(
225225
}
226226
}
227227

228+
// Prevent concurrent task cycling
228229
let taskCycleLock = false;
229230

230231
export async function taskCycleAtPos(pos: number) {
@@ -250,56 +251,58 @@ export async function taskCycleAtPos(pos: number) {
250251
}
251252

252253
export async function taskCycleCommand() {
253-
const text = await editor.getText();
254-
const pos = await editor.getCursor();
255-
const tree = await markdown.parseMarkdown(text);
256-
addParentPointers(tree);
254+
if (taskCycleLock) return;
255+
taskCycleLock = true;
256+
try {
257+
const text = await editor.getText();
258+
const pos = await editor.getCursor();
259+
const tree = await markdown.parseMarkdown(text);
260+
addParentPointers(tree);
257261

258-
let node = nodeAtPos(tree, pos);
259-
if (!node) {
260-
await editor.flashNotification("No task at cursor");
261-
return;
262-
}
263-
if (["BulletList", "Document"].includes(node.type!)) {
264-
// Likely at the end of the line, let's back up a position
265-
node = nodeAtPos(tree, pos - 1);
266-
}
267-
if (!node) {
268-
await editor.flashNotification("No task at cursor");
269-
return;
270-
}
271-
const taskNode =
272-
node.type === "Task"
273-
? node
274-
: findParentMatching(node!, (n) => n.type === "Task");
275-
276-
if (taskNode) {
277-
const taskState = findNodeOfType(taskNode!, "TaskState");
278-
if (taskState) {
279-
// Cycle states: [ ] -> [x] -> (remove checkbox) -> [ ]
280-
await cycleTaskState(taskState, true);
262+
let node = nodeAtPos(tree, pos);
263+
if (!node) {
264+
await editor.flashNotification("No task at cursor");
265+
return;
266+
}
267+
if (["BulletList", "Document"].includes(node.type!)) {
268+
node = nodeAtPos(tree, pos - 1);
269+
}
270+
if (!node) {
271+
await editor.flashNotification("No task at cursor");
272+
return;
273+
}
274+
const taskNode =
275+
node.type === "Task"
276+
? node
277+
: findParentMatching(node!, (n) => n.type === "Task");
278+
279+
if (taskNode) {
280+
const taskState = findNodeOfType(taskNode!, "TaskState");
281+
if (taskState) {
282+
await cycleTaskState(taskState, true);
283+
}
284+
return;
281285
}
282-
return;
283-
}
284286

285-
// Convert a bullet point to a task
286-
const listItem = findParentMatching(node!, (n) => n.type === "ListItem");
287-
if (!listItem) {
288-
await editor.flashNotification("No task at cursor");
289-
return;
290-
}
287+
const listItem = findParentMatching(node!, (n) => n.type === "ListItem");
288+
if (!listItem) {
289+
await editor.flashNotification("No task at cursor");
290+
return;
291+
}
291292

292-
// Check if this ListItem already contains a Task (cursor might be at beginning of line)
293-
const existingTask = findNodeOfType(listItem, "Task");
294-
if (existingTask) {
295-
const taskState = findNodeOfType(existingTask, "TaskState");
296-
if (taskState) {
297-
await cycleTaskState(taskState, true);
293+
const existingTask = findNodeOfType(listItem, "Task");
294+
if (existingTask) {
295+
const taskState = findNodeOfType(existingTask, "TaskState");
296+
if (taskState) {
297+
await cycleTaskState(taskState, true);
298+
}
299+
return;
298300
}
299-
return;
300-
}
301301

302-
await convertListItemToTask(listItem);
302+
await convertListItemToTask(listItem);
303+
} finally {
304+
taskCycleLock = false;
305+
}
303306
}
304307

305308
// Core logic extracted for testability. Mutates tree in place.

0 commit comments

Comments
 (0)