refactor(fileBrowser): rewrite navigation history layer with event-driven NavStack and implement parent directory navigation#2500
Conversation
…iven `NavStack` and implement parent directory navigation
In the legacy file browser implementation, navigation state tracking was heavily coupled with side-effects, manual DOM manipulation, and visual layout updates. Directory historical states were stored in a basic array (`state`) inside local storage and local scopes, and was synchronized through complex manual array operations (such as `.splice()` loops and state array filtering). The logic to handle back navigation and forward rendering on index shifts depended on keeping temporary tracks of prior directories (`lastUrl`, `lastName`) and executing manual nested loops to traverse the DOM tree elements to clear obsolete visual buttons.
This tight coupling introduced several fragility factors:
1. **Race Conditions & Desynchronization**: Slicing or mutating historical arrays manually made it easy for the visual breadcrumbs (navbar buttons) to drift out of sync with the true active folder state.
2. **Brittle Back-Navigation**: Managing navigation events required passing arbitrary callback actions or manual back-navigation tracker arrays (`backNavigation`), which became excessively complicated to maintain when dealing with deeply nested virtual, remote (FTP/SFTP), and local directory structures.
3. **Hard-to-Maintain Boilerplate**: Visual state cleanup on backward traversal required nested manual loops that explicitly inspected the last child element of the navigation bar, mixed with side effects writing directly to `localStorage`.
4. **Absence of O(1) History Validation**: Verifying whether a path was already present in the active navigation history required scanning arrays manually, resulting in poor performance on deep hierarchies.
To resolve these architectural limitations, this refactor introduces a completely encapsulated, event-driven history management class called `NavStack`, which decouples the file browser's state mutations from its representation in the DOM. By treating the history stack as an independent state machine that emits custom events, visual components can independently bind to these events to update themselves automatically.
Additionally, this change introduces robust parent directory navigation support ("one-dir-up" via the classic `..` item) and significantly hardens selection mode operations to prevent non-file actions from being selected.
---
The core architectural change is the implementation of a modern `NavStack` class that inherits from `EventTarget`. Moving to a native class pattern allows all navigation history properties and mutations to remain fully isolated, exposing a predictable API to the file browser:
* **Set-backed Array Tracking**: The class maintains both a private array (`#arr`) of Location objects to preserve order and index-based retrieval, and a private Set (`#urlSet`) containing all active path URIs. This hybrid data structure provides immediate, O(1) checks for path membership (`has(url)`) while maintaining sequential index integrity.
* **Self-Sanitizing Stack Push**: The `.push()` method ensures state duplicate avoidance. It normalizes incoming URIs, checks the internal `Set` for membership, and skips execution if the path is already registered. If the path is unique, it is added to both the array and the set, and dispatches a `"push"` CustomEvent.
* **Stateful Sequential Pop**: The `#popUntil()` and `.pop()` procedures sequentially peel items from the top of the stack, cleanly removing them from the set-backing index and dispatching localized `"pop"` events containing the removed directory metadata. This allows listeners to act specifically on deleted paths as they are removed.
* **Relative Index Retrieval**: The `.get(i)` method provides clean element lookups. It converts indexes using numeric evaluation, supports negative indexing (allowing quick retrieval of relative nodes, e.g., `.get(-1)` for active, and `.get(-2)` for parent nodes) and returns deep copies to prevent external mutations.
* **Event Delegation**: By extending `EventTarget`, other parts of the system can easily register to state updates using shorthand methods `.on()` and `.off()`.
With the introduction of `NavStack`, the file browser controller shifts from manual visual synchronization to an observer-pattern model. Inside the page inclusion logic, a global `navStack` instance is instantiated.
* **Push Synchronization Listener**: Instead of manually building breadcrumb elements and assembling navigate callbacks during standard folder navigation, a centralized `"push"` listener handles the visual injection on the navigation bar. It identifies the immediate previous directory using `.get(-2)` and automatically sets up the recursive directory navigation action.
* **Pop Synchronization Listener**: When a directory path is popped from the stack (whether during back button clicks or navigation index clicks), the `"pop"` listener automatically catches the event details, deletes the bound action stack reference, and removes the associated visual navbar element from the DOM using its unique ID hash (`getNavId(url)`).
* **Persistence Handlers**: The storage of browser history in local storage is now cleanly bound directly to the stack's change events. If `doesOpenLast` is configured, saving to `localStorage.fileBrowserState` is executed automatically on `"push"` or `"pop"` occurrences, ensuring immediate durability.
To align with standard terminal and desktop file management paradigms, this change integrates a dedicated parent folder navigation item `".."` as the first item in directory listings.
* **Prefixing List Entries**: When executing directory reads inside `getDir()`, if the environment is a valid storage provider that supports listing files, a `oneDirUp` flag is toggled. Upon list load, `util.pushFolder()` creates a virtual item named `".."` with custom flags (`oneDirUp: true`, `notSelectable: true`).
* **Array Reordering**: The list modifications move the parent directory tile from the end of the loaded list to the absolute top of the index (`list.unshift(list.pop())`), ensuring the `..` folder tile is always presented at the top of the folder list.
* **Navigation Routing**: Clicking on a `..` tile triggers a dedicated `"oneDirUp"` action case, which retrieves the second-to-last item on the stack via `navStack.get(-2)` and navigates to it, facilitating seamless upward navigation across both local and remote filesystems.
In selection mode, users can perform batch operations on files and folders. However, helper tiles and navigation buttons are not target files. This commit introduces explicit safeguards to keep these tiles outside selection flows:
* **Not-Selectable Flagging**: Tiles like the parent navigation directory `..`, the "Add a Storage" notification banner, and the "Select document" file browser action are injected with a `notSelectable` attribute.
* **Multi-Select Interception**: In selection mode, during checkbox initialization or item selection, elements with `data-not-selectable` are explicitly skipped. This avoids rendering checkbox overlays on top of the parent directory `..` or action lists, preventing invalid selections.
* **Click Prevention**: When selection mode is active, the click handler immediately intercepts events targeted at non-selectable tiles, discarding the action and preventing accidental multi-select inclusions.
* **Long-Press Deletion Safety**: Intercepts delete operations to ensure that attempting to delete or uninstall an item is ignored if the item represents parent navigation (`isOneDirUp`) or file explorer document action overlays.
To align with modern clean-code standards and reduce code complexity, structural processes like `addStorage` have been converted from traditional nested Promise chains (`.then().catch()`) to contemporary `async`/`await` block structures wrapped in robust `try`/`catch` clauses.
---
* **`NavStack` Definition**: Added a complete class definition for `NavStack` at the top of the module, extending `EventTarget` and utilizing private fields (`#urlSet`, `#arr`) to achieve O(1) validation capabilities.
* **`navStack` Initialization**: Removed the unstructured local variable `state` array and replaced it with an active `navStack` class instance.
* **Sync Logic Refactoring**: Incorporated centralized listeners for the `"push"` and `"pop"` events emitted by the stack instance to cleanly drive DOM mutations on the navigation bar, handling `actionStack` actions dynamically.
* **Selection Interception**: Added protective statements inside `querySelectorAll` loop filters to immediately return if the target tile possesses `data-not-selectable`.
* **Interception of Actions**: Modified the global click listener on `$content` tiles to extract `isOneDirUp` from dataset attributes, route parent navigation actions into the `"oneDirUp"` execution block, and guard deletion checks against parent items.
* **Folder Listing Updates**: Updated `getDir()` to track and append a virtual `".."` directory tile, sorting list contents, and using `.unshift()` to enforce placing the directory at index `0`.
* **`navigate` Method Simplification**: Extensively simplified the core navigation routine, deleting nested DOM-removal loops, manual local storage updates, and manual arrays, replacing them with a single execution of `navStack.popUntil()` and `navStack.push()`.
* **`loadStates` Structural Cleanup**: Eliminated elaborate while-loops, back-navigation variables, and redundant state push functions in favor of sequential stack hydration via array shifts.
* **`addStorage` Rewrite**: Migrated the storage addition handler to asynchronous `try/catch` scopes.
* **Template Directives**: Added semantic Mustache tags representing `{{#notSelectable}}data-not-selectable{{/notSelectable}}` and `{{#oneDirUp}}data-one-dir-up{{/oneDirUp}}` properties inside the standard `<li>` file-list tiles. This binds the custom dataset attributes needed for Javascript click routing and selection exclusion checks.
---
1. **Strict State Isolation**: By consolidating navigation history into its own class, the file browser's memory footprint is more manageable, and tracking state shifts is straightforward.
2. **Visual and Data Decoupling**: Removing coupled UI updates from standard navigation functions makes state transitions safe and consistent.
3. **Correctness in Multi-Selection**: Preventing action tiles from participating in multi-select pipelines resolves bugs where system action tiles could be selected during batch operations.
4. **Improved Usability**: Adding parent directory tiles (`..`) simplifies keyboard, mouse, and touch traversal, eliminating the need to rely exclusively on small back-navigation header icons.
5. **Performance Optimization**: O(1) set tracking for active directories reduces lookups to a single hash map index evaluation.
(AI generated commit message)
| function loadStates(states) { | ||
| if (!Array.isArray(states) || !states.length) return; | ||
|
|
||
| const backNavigation = []; | ||
| const lastState = states.pop(); | ||
| if (!lastState || !lastState.url) return; | ||
| const { url } = lastState; | ||
| const name = lastState.name || Url.basename(url) || url; | ||
| let { url: lastUrl, name: lastName } = currentDir; | ||
|
|
||
| while (states.length) { | ||
| const location = states.splice(0, 1)[0]; | ||
| if (!location || !location.url) { | ||
| continue; | ||
| } | ||
| const { url, name } = location; | ||
| let action; | ||
|
|
||
| if (doesOpenLast) pushState({ name, url }); | ||
| if (lastUrl && lastName) { | ||
| backNavigation.push([lastUrl, lastName]); | ||
| action = () => { | ||
| const [url, name] = backNavigation.pop(); | ||
| navigate(url, name, false); | ||
| }; | ||
| } | ||
| pushToNavbar(name, url, action); | ||
| lastUrl = url; | ||
| lastName = name; | ||
| } | ||
|
|
||
| currentDir = { url: lastUrl, name: lastName }; | ||
| while (states.length) navStack.push(states.shift()); | ||
| currentDir = navStack.get(-1); | ||
| const { url, name } = currentDir; | ||
| navigate(url, name); |
There was a problem hiding this comment.
loadStates silently drops the last stored directory when name is falsy
The old implementation had a defensive fallback: const name = lastState.name || Url.basename(url) || url. The new code calls navStack.get(-1).name directly and passes it to navigate, which throws synchronously if name is falsy. Because loadStates does not await or .catch() the call, the resulting Promise rejection is unhandled — the navbar is populated (via the push-event loop) but the final directory is never rendered and its content never shown. Any stored state object whose name was somehow saved as null, "", or undefined silently breaks restore-on-open.
| case "oneDirUp": { | ||
| const dir = navStack.get(-2); | ||
| if (!dir) break; | ||
| const { url, name } = dir; | ||
| navigate(url, name); | ||
| } |
There was a problem hiding this comment.
.. resolves to navigation-history parent, not the filesystem parent
navStack.get(-2) returns the previously-visited directory, not the actual URL-parent of the current directory. These are the same in linear navigation, but diverge in edge cases — e.g. if a future feature adds bookmarks or deep-links that push multiple levels to navStack at once (like loadStates already does). In that scenario pressing .. could land on a directory that is not an ancestor of the current one at all. The traditional expected behaviour of .. is Url.dirname(currentDir.url). Consider adding a clarifying comment or computing the real parent as a fallback.
| if (isSelectionMode) { | ||
| if ($el.dataset.notSelectable != null) return; |
There was a problem hiding this comment.
data-not-selectable guard misses clicks on child elements
$el is e.target, which can be the icon <span> or text <div> inside the <li> rather than the <li> itself. Neither child carries data-not-selectable, so $el.dataset.notSelectable != null evaluates to false and the early-return guard is skipped. The .querySelector(".input-checkbox") fallback happens to save correctness here (notSelectable items never get a checkbox added), but the safety relies on that second check rather than the explicit guard. Replacing with $el.closest(".tile")?.dataset.notSelectable != null would make the check authoritative for any click target depth.
| case "oneDirUp": { | ||
| const dir = navStack.get(-2); | ||
| if (!dir) break; | ||
| const { url, name } = dir; | ||
| navigate(url, name); | ||
| } |
There was a problem hiding this comment.
Missing
break at end of oneDirUp case
The oneDirUp block has no trailing break. While this is currently safe because it is the last case, future additions to the switch will silently fall through into the new case without any visible indication that the omission is intentional. Adding break makes the intent explicit and future-proof.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Context & Motivation
In the legacy file browser implementation, navigation state tracking was heavily coupled with side-effects, manual DOM manipulation, and visual layout updates. Directory historical states were stored in a basic array (
state) inside local storage and local scopes, and was synchronized through complex manual array operations (such as.splice()loops and state array filtering). The logic to handle back navigation and forward rendering on index shifts depended on keeping temporary tracks of prior directories (lastUrl,lastName) and executing manual nested loops to traverse the DOM tree elements to clear obsolete visual buttons.This tight coupling introduced several fragility factors:
backNavigation), which became excessively complicated to maintain when dealing with deeply nested virtual, remote (FTP/SFTP), and local directory structures.localStorage.To resolve these architectural limitations, this refactor introduces a completely encapsulated, event-driven history management class called
NavStack, which decouples the file browser's state mutations from its representation in the DOM. By treating the history stack as an independent state machine that emits custom events, visual components can independently bind to these events to update themselves automatically.Additionally, this change introduces robust parent directory navigation support ("one-dir-up" via the classic
..item) and significantly hardens selection mode operations to prevent non-file actions from being selected.Architectural Design and Structural Modifications
1. Introduction of the
NavStackClassThe core architectural change is the implementation of a modern
NavStackclass that inherits fromEventTarget. Moving to a native class pattern allows all navigation history properties and mutations to remain fully isolated, exposing a predictable API to the file browser:#arr) of Location objects to preserve order and index-based retrieval, and a private Set (#urlSet) containing all active path URIs. This hybrid data structure provides immediate, O(1) checks for path membership (has(url)) while maintaining sequential index integrity..push()method ensures state duplicate avoidance. It normalizes incoming URIs, checks the internalSetfor membership, and skips execution if the path is already registered. If the path is unique, it is added to both the array and the set, and dispatches a"push"CustomEvent.#popUntil()and.pop()procedures sequentially peel items from the top of the stack, cleanly removing them from the set-backing index and dispatching localized"pop"events containing the removed directory metadata. This allows listeners to act specifically on deleted paths as they are removed..get(i)method provides clean element lookups. It converts indexes using numeric evaluation, supports negative indexing (allowing quick retrieval of relative nodes, e.g.,.get(-1)for active, and.get(-2)for parent nodes) and returns deep copies to prevent external mutations.EventTarget, other parts of the system can easily register to state updates using shorthand methods.on()and.off().2. Event-Driven UI Synchronization & Loose Coupling
With the introduction of
NavStack, the file browser controller shifts from manual visual synchronization to an observer-pattern model. Inside the page inclusion logic, a globalnavStackinstance is instantiated."push"listener handles the visual injection on the navigation bar. It identifies the immediate previous directory using.get(-2)and automatically sets up the recursive directory navigation action."pop"listener automatically catches the event details, deletes the bound action stack reference, and removes the associated visual navbar element from the DOM using its unique ID hash (getNavId(url)).doesOpenLastis configured, saving tolocalStorage.fileBrowserStateis executed automatically on"push"or"pop"occurrences, ensuring immediate durability.3. Support for Parent Directory Navigation ("One Dir Up")
To align with standard terminal and desktop file management paradigms, this change integrates a dedicated parent folder navigation item
".."as the first item in directory listings.getDir(), if the environment is a valid storage provider that supports listing files, aoneDirUpflag is toggled. Upon list load,util.pushFolder()creates a virtual item named".."with custom flags (oneDirUp: true,notSelectable: true).list.unshift(list.pop())), ensuring the..folder tile is always presented at the top of the folder list...tile triggers a dedicated"oneDirUp"action case, which retrieves the second-to-last item on the stack vianavStack.get(-2)and navigates to it, facilitating seamless upward navigation across both local and remote filesystems.4. Hardening Selection Mode and Bypassing Interactive Tiles
In selection mode, users can perform batch operations on files and folders. However, helper tiles and navigation buttons are not target files. This commit introduces explicit safeguards to keep these tiles outside selection flows:
.., the "Add a Storage" notification banner, and the "Select document" file browser action are injected with anotSelectableattribute.data-not-selectableare explicitly skipped. This avoids rendering checkbox overlays on top of the parent directory..or action lists, preventing invalid selections.isOneDirUp) or file explorer document action overlays.5. Promise to Async/Await Modernization
To align with modern clean-code standards and reduce code complexity, structural processes like
addStoragehave been converted from traditional nested Promise chains (.then().catch()) to contemporaryasync/awaitblock structures wrapped in robusttry/catchclauses.File-by-File Changes Description
1.
src/pages/fileBrowser/fileBrowser.jsNavStackDefinition: Added a complete class definition forNavStackat the top of the module, extendingEventTargetand utilizing private fields (#urlSet,#arr) to achieve O(1) validation capabilities.navStackInitialization: Removed the unstructured local variablestatearray and replaced it with an activenavStackclass instance."push"and"pop"events emitted by the stack instance to cleanly drive DOM mutations on the navigation bar, handlingactionStackactions dynamically.querySelectorAllloop filters to immediately return if the target tile possessesdata-not-selectable.$contenttiles to extractisOneDirUpfrom dataset attributes, route parent navigation actions into the"oneDirUp"execution block, and guard deletion checks against parent items.getDir()to track and append a virtual".."directory tile, sorting list contents, and using.unshift()to enforce placing the directory at index0.navigateMethod Simplification: Extensively simplified the core navigation routine, deleting nested DOM-removal loops, manual local storage updates, and manual arrays, replacing them with a single execution ofnavStack.popUntil()andnavStack.push().loadStatesStructural Cleanup: Eliminated elaborate while-loops, back-navigation variables, and redundant state push functions in favor of sequential stack hydration via array shifts.addStorageRewrite: Migrated the storage addition handler to asynchronoustry/catchscopes.2.
src/pages/fileBrowser/list.hbs{{#notSelectable}}data-not-selectable{{/notSelectable}}and{{#oneDirUp}}data-one-dir-up{{/oneDirUp}}properties inside the standard<li>file-list tiles. This binds the custom dataset attributes needed for Javascript click routing and selection exclusion checks.Engineering Impact and System Stability
..) simplifies keyboard, mouse, and touch traversal, eliminating the need to rely exclusively on small back-navigation header icons.(PR name and description are AI generated)