Skip to content

Fix: Bug: Arrow navigation jumps to top in preview mode - #173

Closed
over-fix-agent-dev[bot] wants to merge 1 commit into
masterfrom
over/bug-1007-46032d15
Closed

over-fix-agent-dev[bot] wants to merge 1 commit into
masterfrom
over/bug-1007-46032d15

Conversation

@over-fix-agent-dev

Copy link
Copy Markdown

Bug

Bug: Arrow navigation jumps to top in preview mode

Fix the preview mode arrow navigation bug based on previous findings in ui/main_logic.js

Cause

In ui/main_logic.js the global document mousedown handler (around lines 703–766) calls unSelectAllItems() for any click that isn't on a whitelisted element. That call sets SelectedElement to null. The whitelist had no entry for .item-preview-popup. It also only checked e.target.classList for item parts, so clicks on some child elements inside an .item-link entry also cleared the selection. When SelectedElement is null, the nav_down (and nav_up) keyboard handler (around line 1092) calls goUp(false, true). That selects item index 0, so the selection jumps to the top. It only happens after a click lands on one of these elements, which is why it happens "often but not always".

Fix

I added two checks to the click-outside condition of the global mousedown handler in ui/main_logic.js: !e.target.closest(".item-preview-popup") and !e.target.closest(".item-link"). Clicking inside the preview popup, or anywhere on a file or folder entry (including its child elements), no longer resets the selection. SelectedElement stays set, and ArrowDown/ArrowUp move on from the selected item instead of starting over at the first one. Item clicks still reset the rest of the selection in the normal way through the item's onclick → interactWithItem → selectItem. I didn't change anything else.

Tests

node --check ui/main_logic.js passes, so the syntax is valid. The project has no JS test suite or package.json for the frontend, so I added no test and haven't tested it in the running app.

Changed files

  • ui/main_logic.js (+4 −0)

Opened by the Over fix agent for task #1007. It was not merged: please review it like any other change.

I added two checks to the click-outside condition of the global mousedown handler in ui/main_logic.js: `!e.target.closest(".item-preview-popup")` and `!e.target.closest(".item-link")`. Clicking inside the preview popup, or anywhere on a file or folder entry (including its child elements), no longer resets the selection. SelectedElement stays set, and ArrowDown/ArrowUp move on from the selected item instead of starting over at the first one. Item clicks still reset the rest of the selection in the normal way through the item's onclick → interactWithItem → selectItem. I didn't change anything else.

Found and fixed by the Over fix agent for task #1007.
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 90a2d477-dddb-44f4-9e46-8df316dc3171

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

Comment @coderabbitai help to get the list of available commands.

@RickyDane

Copy link
Copy Markdown
Owner

Bitte begründe deine Entscheidungen und warum die Änderungen so korrekt sind, wie du sie gemacht hast

@over-fix-agent-dev

Copy link
Copy Markdown
Author

Warum die Änderung so aussieht

Der globale mousedown-Handler in ui/main_logic.js (Z. 703–766) arbeitet mit einer Ausschlussliste. Trifft ein Klick keines der aufgeführten Elemente, laufen cdCtMenu.hide(), resetContextMenu() und unSelectAllItems() (Z. 754–761). Wenn danach SelectedElement == null ist, rufen nav_up/nav_down goUp(false, true) auf (Z. 1086–1087 bzw. 1096–1097). Dadurch springt die Auswahl ganz nach oben. Das ist der gemeldete Fehler.

Was vorher passiert ist

  • Das Vorschau-Popup .item-preview-popup stand nicht auf der Liste. Von seinem Inhalt waren nur einzelne Klassen ausgenommen (item-preview-file-content, popup-header, item-preview-copy-path-button, Z. 741–743). Ein Klick auf eine andere Stelle im Popup hat also die Auswahl gelöscht.
  • Die Einträge selbst wurden nur per classList.contains geprüft, also nur für das direkte Ziel (Z. 731–740). Ein Klick auf ein Kind-Element von .item-link, das nicht auf der Liste steht, hat die Auswahl ebenfalls gelöscht. Deshalb trat der Fehler „oft, aber nicht immer“ auf.

Was die Änderung abdeckt

Z. 721–722 prüfen jetzt mit closest(). Damit gilt jeder Klick innerhalb des Popups oder irgendwo in einem Eintrag als „innen“. Das passt zum bestehenden Muster bei .context-menu, .props-card und .site-nav-bar-button (Z. 716–718, 748). Klicks auf freie Flächen setzen die Auswahl weiterhin zurück.

Wo ich unsicher bin bzw. wo du einen Punkt hast

  • Ich habe die Definition von unSelectAllItems() und die onclick-Kette interactWithItem → selectItem bei dieser Antwort nicht noch einmal gelesen. Die Suche indexiert main_logic.js (12 220 Zeilen) nicht. Dass ein Klick auf einen Eintrag die übrige Auswahl über den eigenen Handler korrekt zurücksetzt, ist also eine Annahme, keine geprüfte Aussage.
  • Nebeneffekt: Bei Klicks auf Kind-Elemente eines Eintrags, die vorher nicht ausgenommen waren, schließt dieser Handler das Kontextmenü jetzt nicht mehr. Außerdem setzt er den DraggedOverElement-Filter nicht mehr zurück (Z. 754–764). Für die bereits ausgenommenen Klassen wie item-button oder item-icon war das schon vorher so. Neu ist es nur für die bisher nicht erfassten Kinder.
  • Die alten classList-Prüfungen für Eintrags-Klassen sind jetzt zum Teil überflüssig. Ich habe sie bewusst stehen lassen, damit der Diff klein bleibt.
  • Getestet ist nur die Syntax (node --check), nicht das Verhalten in der laufenden App.

@RickyDane
RickyDane marked this pull request as ready for review September 27, 2026 19:44
@RickyDane

Copy link
Copy Markdown
Owner

Bitte teste das nochmal und melde dann nochmal zurück

@over-fix-agent-dev

Copy link
Copy Markdown
Author

Working on this feedback now.

@over-fix-agent-dev

Copy link
Copy Markdown
Author

I could not address this feedback, so it needs a person.

The agent reported a fix but did not change any file.

Summary: Review point, "Bitte teste das nochmal und melde dann nochmal zurück" (please test again and report back): I tested it again. No code change was needed and the branch is unchanged.

How I tested: the repo has no JS test suite, and the full Tauri app can't run in the sandbox. So I wrote a throwaway jsdom harness outside the repo (not committed). It reads the actual click-outside condition straight from ui/main_logic.js (lines 715–752), so it tests the code as it is on this branch. It also builds the pre-fix condition by removing the two new closest() lines. It then replays the reported steps: click item 3 (selectItem), press Space (the preview opens because SelectedElement != null), a mousedown lands somewhere, then ArrowDown. In ArrowDown, lines 1094–1102 call goUp(false, true), which jumps to the first item, whenever SelectedElement is null. Otherwise they call goGridDown/goDown from the current index. The DOM uses the real item markup from createItemInnerHtml (wrap and column views) and the preview popup markup.

Results:

  • Old code: the jump to the top happens in both views when the mousedown lands on the preview popup, its header <h3>, .module-container or preview <img>. It also happens on an item's .preloader-small-invert spinner. That spinner is only visible while a thumbnail (e.g. an image) is still loading, which fits the report that the bug is intermittent but frequent.
  • New code: every one of those cases now moves to the next item (index 7 in wrap with 4 per row, index 4 in column). Clicks on the item button, icon, text and size box keep working as before.
  • A click on empty space (explorer container or body) still clears the selection, so the normal "click outside to deselect" behaviour is intact.

Limitation: I could only simulate this in a DOM, not click through the real app. Ricky, a quick manual check in the built app would confirm it. Note: the single-pane goDown/goUp column-mode functions only handle SelectedItemPaneSide "left"/"right". I saw this while tracing but it's outside this bug, so I didn't touch it.
Cause: The global mousedown handler in ui/main_logic.js (click-outside check) called unSelectAllItems() and set SelectedElement = null for clicks inside .item-preview-popup and on item child elements it didn't list, such as .preloader-small-invert. The next ArrowDown then went through the SelectedElement == null branch into goUp(false, true) and restarted at the first item. The earlier fix adds !e.target.closest(".item-preview-popup") and !e.target.closest(".item-link") to that check.
Tests: node --check ui/main_logic.js: syntax OK. A jsdom harness (installed in /tmp, not committed) that evaluates the real condition taken from ui/main_logic.js: 18 click targets across wrap and column views plus the empty-area check. Old code jumps to the top for 10 targets; the new code passes all of them ("ALL NEW-CODE CHECKS PASSED"), and a click on empty sp
… (cut off)

@RickyDane RickyDane closed this Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant