Desktop: Resolves #15029 Fix: Prevent unexpected focus shift when switching notebooks or using Go To

[justin212407]

Fixes: #15029

Summary:

When navigating between notebooks using keyboard arrow keys, focus unexpectedly shifts from the notebook list to the notes list after a few seconds. Additionally, the "Go To" feature exhibits inconsistent behavior where focus sometimes lands on the notes list instead of the selected note.

Fix:

The useRefocusOnDeletion hook was incorrectly refocusing during notebook folder switches due to lagging state updates across render cycles. When users navigated to a different folder using keyboard, the folder ID and note count updated on separate renders. By the time the note count updated, the usePrevious hook had already captured the new folder ID, causing the hook to mistakenly believe a note was deleted in the same folder when in fact the folder had switched.

Updated focus refocus logic to:

• Track the folder associated with the last note-count change using a stable useRef instead of the lagging usePrevious hook
• Only update the ref when the note count actually changes, ensuring the guard depends on consistent state history
• Filter refocus to only occur when a note is genuinely deleted within the same folder, blocking false positives during folder transitions
• Maintain all existing single-note deletion behavior and backward compatibility

Changes Made

packages/app-desktop/gui/NoteList/utils/useRefocusOnDeletion.ts:

• Imported useRef from React to create a stable lastFolderIdRef
• Replaced usePrevious(selectedFolderId, '') with useRef(selectedFolderId) to track the folder at the last note-count snapshot
• Updated folderDidNotChange guard to compare against lastFolderIdRef.current instead of the lagging previous folder value
• Added logic to update lastFolderIdRef.current only when the note count changes, ensuring the ref remains synchronized with the snapshot point
• Removed previousFolderId from the effect dependency array to stabilize the guard

packages/app-desktop/gui/NoteList/utils/useRefocusOnDeletion.test.ts:

• Enhanced regression test case to reproduce the two-rerender folder-switch scenario: first rerender changes folder while preserving note count, second rerender decreases note count in the new folder
• Updated test description to clarify the multi-rerender timing requirement
• Verified all existing tests (single-line deletion, note count increase, other field focus, multiple note selection) continue to pass

Testing:

BEFORE:

Screencast.From.2026-04-06.20-34-08.mp4

AFTER:

Screencast.From.2026-04-07.02-22-04.mp4

All five test cases pass, including:

• Refocus behavior still works when a note is deleted within the same folder
• No refocus when note count increases
• No refocus when editing another field
• No refocus when multiple notes are selected
• No refocus when switching to a folder with fewer notes across separate render cycles

AI Disclosure:

AI was used to make changes to the test.
AI was used to make the vocabulary for the PR description.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 81be5d8a-473e-4475-b816-c214a4a7af1c

📥 Commits

Reviewing files that changed from the base of the PR and between 850b9db and 6c36018.

📒 Files selected for processing (1)

• packages/app-desktop/gui/NoteList/utils/useRefocusOnDeletion.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/app-desktop/gui/NoteList/utils/useRefocusOnDeletion.test.ts

---

📝 Walkthrough

Walkthrough

Refactors useRefocusOnDeletion to track folder snapshots via internal refs and require a DOM listRef for focus checks; updates NoteList to pass listRef and focusNote; tests rewritten to use a createFocusContext helper and explicitly set DOM focus across rerenders, including multi-step folder-change scenarios.

Changes

Cohort / File(s)	Summary
Hook refactor packages/app-desktop/gui/NoteList/utils/useRefocusOnDeletion.ts	Replaced usePrevious with lastNoteCountFolderRef and folderChangedSinceLastNoteCountRef; added effect to mark folder changes; compute noteListHasFocus via listRef.current.contains(document.activeElement); updated refocus gating and changed hook signature to accept listRef and focusNote.
Tests updated packages/app-desktop/gui/NoteList/utils/useRefocusOnDeletion.test.ts	Introduced createFocusContext helper that mounts/removes list and editor elements; updated tests to set focus via the helper before rerenders; changed hook invocation order to pass listRef before focusNote; added multi-rerender folder-switch and editor-focused cases; ensured cleanup with try/finally.
Caller update packages/app-desktop/gui/NoteList/NoteList2.tsx	Updated call to useRefocusOnDeletion to pass the new listRef and focusNote arguments to match the refactored hook signature.

Sequence Diagram(s)

sequenceDiagram
    participant NoteList as NoteList component
    participant Hook as useRefocusOnDeletion
    participant DOM as Document / ActiveElement
    participant Focus as focusNote callback

    NoteList->>Hook: call useRefocusOnDeletion(noteCount, selectedNoteIds, focusedField, selectedFolderId, listRef, focusNote)
    NoteList->>Hook: noteCount changes (deletion)
    Hook->>Hook: update lastNoteCountFolderRef and reset folderChangedSinceLastNoteCountRef
    Hook->>DOM: evaluate listRef.current.contains(document.activeElement) as noteListHasFocus
    alt note removed && folder unchanged since snapshot && one selected && no field focused && noteListHasFocus
        Hook->>Focus: call focusNote(selectedNoteId)
    else
        Hook-->>NoteList: do not call focusNote
    end

Loading

Possibly related PRs

• Desktop: Fix note list losing focus after moving note to trash #14650: Modifies the same useRefocusOnDeletion hook and its tests; closely related changes to refocus logic and test coverage.

Suggested reviewers

• personalizedrefrigerator

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly references the issue being fixed (#15029) and succinctly summarises the main change: preventing unexpected focus shifts when switching notebooks or using Go To.
Description check	✅ Passed	The description thoroughly explains the problem, root cause, solution, and changes made, all directly related to the changeset.
Linked Issues check	✅ Passed	The PR successfully addresses the core objectives from issue #15029 by preventing unwanted focus shifts from the notebook list to the notes list during keyboard navigation through folder changes.
Out of Scope Changes check	✅ Passed	All changes remain focused on fixing the useRefocusOnDeletion hook logic; there are no unrelated modifications outside the scope of addressing the focus-shift issue.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Pr Description Must Follow Guidelines	✅ Passed	The PR description comprehensively follows all required GSoC guidelines with three mandatory sections: Problem/User-Impact, High-Level Solution with key changes, and Test Plan/Verification steps.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[personalizedrefrigerator]

fix: add two refs to track folder changed and last note count snapshot

Thanks! 2f61267 seems to have fixed one aspect of the issue. However, I'm still able to reproduce the focus jump issue in a different case:

1. Set up a background task that adds/removes notes.
   • This might be done by sync or a plugin.
   • I did this by running a script in Joplin's development tools that repeatedly adds a note, then deletes it, from a folder.
2. Focus the editor and start typing.
3. Observe that focus occasionally jumps to the notes list while editing.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

packages/app-desktop/gui/NoteList/utils/useRefocusOnDeletion.ts (1)

15-20: ⚠️ Potential issue | 🟠 Major

Same-count folder switches still leave this guard stale.

If the user switches folders and both renders start with the same noteCount, Line 18 sets folderChangedSinceLastNoteCountRef.current to true, but Lines 34-36 never clear it because the count did not change. The next genuine deletion in the new folder is then treated as a folder-switch artefact, so focusNote does not run.

Also applies to: 33-37

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/app-desktop/gui/NoteList/utils/useRefocusOnDeletion.ts` around lines
15 - 20, When selectedFolderId changes we currently set
folderChangedSinceLastNoteCountRef.current = true but if the new folder has the
same noteCount the effect that clears that flag (which compares
lastNoteCountRef.current to noteCount) never runs; fix by also updating the
snapshot refs on folder switch: in the useEffect watching selectedFolderId (in
useRefocusOnDeletion.ts) set lastNoteCountFolderRef.current = selectedFolderId
and lastNoteCountRef.current = noteCount (in addition to
folderChangedSinceLastNoteCountRef.current = true) so the subsequent deletion
logic doesn’t treat the first delete in the new folder as a folder-switch
artefact.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/app-desktop/gui/NoteList/utils/useRefocusOnDeletion.test.ts`:
- Around line 87-99: The test never gives the note list focus so
noteListHasFocus stays false and prevents focusNote from being called; before
the rerenders in the test that uses createFocusContext(), renderHook(...
useRefocusOnDeletion ...), and focusNote, programmatically focus the list
element (via the returned listRef or listItem from createFocusContext) so the
component thinks the list has focus (e.g., call focus on listRef.current or fire
a focus event) prior to the two rerenders; this ensures useRefocusOnDeletion
sees the focused state and will exercise the folder-switch guard that should
prevent refocusing.

---

Duplicate comments:
In `@packages/app-desktop/gui/NoteList/utils/useRefocusOnDeletion.ts`:
- Around line 15-20: When selectedFolderId changes we currently set
folderChangedSinceLastNoteCountRef.current = true but if the new folder has the
same noteCount the effect that clears that flag (which compares
lastNoteCountRef.current to noteCount) never runs; fix by also updating the
snapshot refs on folder switch: in the useEffect watching selectedFolderId (in
useRefocusOnDeletion.ts) set lastNoteCountFolderRef.current = selectedFolderId
and lastNoteCountRef.current = noteCount (in addition to
folderChangedSinceLastNoteCountRef.current = true) so the subsequent deletion
logic doesn’t treat the first delete in the new folder as a folder-switch
artefact.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8a0a765f-4a86-4655-a3f3-d2cdbb870310

📥 Commits

Reviewing files that changed from the base of the PR and between 2f61267 and 850b9db.

📒 Files selected for processing (3)

• packages/app-desktop/gui/NoteList/NoteList2.tsx
• packages/app-desktop/gui/NoteList/utils/useRefocusOnDeletion.test.ts
• packages/app-desktop/gui/NoteList/utils/useRefocusOnDeletion.ts

[justin212407]

fix: add two refs to track folder changed and last note count snapshot

Thanks! 2f61267 seems to have fixed one aspect of the issue. However, I'm still able to reproduce the focus jump issue in a different case:

1. Set up a background task that adds/removes notes.

   • This might be done by sync or a plugin.
   • I did this by running a script in Joplin's development tools that repeatedly adds a note, then deletes it, from a folder.

2. Focus the editor and start typing.

3. Observe that focus occasionally jumps to the notes list while editing.

I think the edge case you talked about is fixed now after the latest commit. I used the same script in a plugin(it adds a note, then deletes it, from a folder). Here is a working demo of the fix

Screencast.From.2026-04-10.11-24-46.mp4

[personalizedrefrigerator]

Thank you for fixing that!

Now that there's logic to check whether listRef has focus, would it be possible to simplify the hook by removing the !focusedField check?

Additionally, would the hook still work if lastNoteCountFolderRef.current = selectedFolderId; is always set (rather than just if noteCount !== previousNoteCount)?

[justin212407]

I checked this:

Now that there's logic to check whether listRef has focus, would it be possible to simplify the hook by removing the !focusedField check?

Removing !focusedField is possible, but it would change current behavior since it acts as a safeguard alongside DOM focus, and tests rely on it. The existing tests also rely on this (e.g., ensuring we don’t refocus when another field is active), so removing it would require updating that contract.

Additionally, would the hook still work if lastNoteCountFolderRef.current = selectedFolderId; is always set (rather than just if noteCount !== previousNoteCount)?

For lastNoteCountFolderRef.current = selectedFolderId, setting it unconditionally would probably still work in most cases, but the current condition (noteCount !== previousNoteCount) better preserves the idea of tracking changes relative to the last note-count snapshot. It makes folder-switch behavior easier to reason about.

So I’d keep both as-is unless we want to intentionally change behavior and update tests.

[personalizedrefrigerator]

In terms of functionality, this looks good to me!

In terms of style and code structure:

• Using !focusedField in the if condition might be misleading, since !focusedField suggests "no fields are focused", but really seems to only mean "the search field is not focused" (see code search for FOCUS_SET). I think focusedField could be worth removing, even if it means adjusting tests.
• Consider adjusting the When noteCount changes, update snapshot and reset folder-change tracking comment. Currently, it describes what the if statement is doing. It may be more helpful to explain why it is necessary.

[justin212407]

Yeah I think these changes do make a lot of sense. On it right now.

[personalizedrefrigerator]

Be aware that #15029 might be resolved by reverting #14650: If I revert #14650 locally, I'm unable to reproduce its linked issue (#10753) -- the useRefocusOnDeletion hook might not be necessary.

[justin212407]

I will try to revert #14650 locally and check whether #10753 and #15029 still persists. If it does not i also think it might be safe to just revert the changes instead of adding new complexities and hooks to the codebase.