Move focus off a row that has been deleted
Two complaints in #71, one cause. Deleting from the keyboard left `focusId` pointing at a row that was no longer in the list, and nothing moved it. The confirmation appearing "every other message": `targetIds()` falls back to the focused id, so the second `#` re-targeted the message the first one had just deleted. The optimistic update had already moved that message into Deleted Items, so it read as a permanent delete -- and a permanent delete always confirms, whatever "Confirm before deleting" is set to. The dialog was correct about the message it was asked about; it was asked about the wrong one. `k` jumping to the top: `moveFocus` reads `ids.indexOf(focusId)`, which was -1 for the departed row, and -1 is treated as "before the first row". Adding -1 to that clamps to 0. Both explain why the mouse was fine: clicking sets focus to a row that exists. Focus now moves to whatever slid into the deleted row's place, honouring "After archiving or deleting" -- the row below by default, the one above when set to newer -- and clears when the folder empties. `moveFocus` also no longer reads a missing row as index 0; it falls back to where the list thinks it is. Verified in the browser against the mock, since arithmetic tests cannot prove the wiring: with confirmation off, two deletes in a row both go through silently, focus stepping e4 to e5 to e6 as rows close up; then `k` moves up exactly one instead of to the top of the list.
This commit is contained in:
@@ -115,6 +115,37 @@ export function MailView({ mailboxId, threadId, search }: { mailboxId?: string;
|
||||
(removed: boolean) => {
|
||||
useMail.getState().clearSelection();
|
||||
if (!removed) return;
|
||||
|
||||
/*
|
||||
* Move the focused row off the message that just went away.
|
||||
*
|
||||
* Nothing did this before, so `focusId` kept pointing at a row that was
|
||||
* no longer in the list, and two separate complaints in #71 fell out of
|
||||
* it. `targetIds()` falls back to the focused id, so the next `#`
|
||||
* re-targeted the deleted message -- which the optimistic update had
|
||||
* already marked as being in Deleted Items, making it look like a
|
||||
* permanent delete and raising a confirmation the setting had turned
|
||||
* off. And `moveFocus` reads `ids.indexOf(focusId)`, which was -1, which
|
||||
* it treats as "before the start" -- so `k` clamped to the top of the
|
||||
* list.
|
||||
*
|
||||
* Clicking a row was unaffected, because that sets focus to a row that
|
||||
* exists, which is why it only ever happened from the keyboard.
|
||||
*
|
||||
* `currentRowIndex` here is the value from the render that started this
|
||||
* action, so it is the index the message had *before* it was removed.
|
||||
* The row that slid into that slot is the one to focus.
|
||||
*/
|
||||
const wasAt = currentRowIndex;
|
||||
const freshIds = useMail.getState().list?.ids ?? [];
|
||||
if (!freshIds.length) {
|
||||
setFocusId(null);
|
||||
} else if (wasAt >= 0) {
|
||||
const want = settings.autoAdvance === "newer" ? wasAt - 1 : wasAt;
|
||||
const next = freshIds[Math.max(0, Math.min(want, freshIds.length - 1))];
|
||||
if (next) setFocusId(next);
|
||||
}
|
||||
|
||||
// auto-advance
|
||||
if (threadId) {
|
||||
const idx = currentRowIndex;
|
||||
@@ -128,7 +159,7 @@ export function MailView({ mailboxId, threadId, search }: { mailboxId?: string;
|
||||
}
|
||||
}
|
||||
},
|
||||
[threadId, currentRowIndex, settings.autoAdvance, ids, rowThreadId, openThread],
|
||||
[threadId, currentRowIndex, settings.autoAdvance, ids, rowThreadId, openThread, setFocusId],
|
||||
);
|
||||
|
||||
const actions = useMemo(
|
||||
@@ -191,7 +222,11 @@ export function MailView({ mailboxId, threadId, search }: { mailboxId?: string;
|
||||
focusRef.current = focusId;
|
||||
useEffect(() => {
|
||||
const moveFocus = (delta: number) => {
|
||||
const cur = focusRef.current ? ids.indexOf(focusRef.current) : currentRowIndex;
|
||||
// A focused id that is no longer in the list gives -1, which must not be
|
||||
// read as "just before the first row" -- that is what sent `k` to the
|
||||
// top. Fall back to where the list thinks we are instead.
|
||||
const fromFocus = focusRef.current ? ids.indexOf(focusRef.current) : -1;
|
||||
const cur = fromFocus >= 0 ? fromFocus : currentRowIndex;
|
||||
const next = Math.max(0, Math.min(ids.length - 1, (cur < 0 ? (delta > 0 ? -1 : 0) : cur) + delta));
|
||||
const id = ids[next];
|
||||
if (!id) return;
|
||||
|
||||
Reference in New Issue
Block a user