Grid view mode #99

xgrommx · 2025-12-25T18:30:20Z

No description provided.

xgrommx · 2025-12-25T18:57:09Z

Hello @ldm0. I tried to separate Grid View Mode and Advance Search Dialog. Now u can check Grid View Mode

Copilot

Pull request overview

This PR introduces a grid view mode alongside the existing list view, allowing users to visualize search results as a grid of icons with filenames. The implementation includes dynamic icon sizing, marquee selection, responsive layout, and persistence of view preferences.

Key changes include:

Backend icon generation now returns dimensions and supports configurable sizes for optimal rendering in grid view
New VirtualGrid component with marquee selection and keyboard navigation
View mode toggle in the search bar and preferences overlay with icon size slider
File deletion commands (trash and permanent delete) with keyboard shortcuts

Reviewed changes

Copilot reviewed 30 out of 32 changed files in this pull request and generated 20 comments.

Show a summary per file

File	Description
fs-icon/src/lib.rs	Enhanced icon generation to return dimensions and support dynamic sizing with aspect ratio preservation
fs-icon/tests/additional.rs	Updated test to match new icon API signature
cardinal/src/utils/viewPreferences.ts	New utility for persisting view mode and icon size preferences to localStorage
cardinal/src/types/search.ts	Added iconWidth and iconHeight fields to search result types
cardinal/src/types/ipc.ts	Extended icon update payloads with dimension information
cardinal/src/hooks/useSelection.ts	Added bulkSelect for marquee selection and flexible delta for grid navigation
cardinal/src/hooks/useIconViewport.ts	Updated to pass icon size to backend for appropriate icon generation
cardinal/src/hooks/useDataLoader.ts	Enhanced to handle icon dimensions in cache updates
cardinal/src/hooks/useContextMenu.ts	Added trash and delete menu items with handlers
cardinal/src/components/VirtualGrid.tsx	New component implementing grid layout with marquee selection and virtualized rendering
cardinal/src/components/FileGridItem.tsx	New component for rendering individual grid items with icons and labels
cardinal/src/components/VirtualList.tsx	Refactored positioning to use transform for consistency with VirtualGrid
cardinal/src/components/StatusBar.tsx	Added icon size slider control for grid view
cardinal/src/components/SearchBar.tsx	Added view mode toggle buttons
cardinal/src/components/PreferencesOverlay.tsx	Added default view mode and icon size preferences
cardinal/src/components/FilesTabContent.tsx	Modified to conditionally render VirtualList or VirtualGrid based on view mode
cardinal/src/components/FileRow.tsx	Refactored drag handling to use callback pattern
cardinal/src/App.tsx	Integrated view mode state, keyboard navigation for grid, and file deletion commands
cardinal/src/App.css	Added comprehensive styling for grid view, view mode toggle, and icon size slider
cardinal/src-tauri/src/commands.rs	Added trash_paths and delete_paths commands, updated icon viewport with size parameter
cardinal/src-tauri/src/background.rs	Updated icon processing to include dimensions
cardinal/src-tauri/Cargo.toml	Added trash crate dependency
cardinal/src/i18n/resources/en-US.json	Added translations for new UI elements

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-27T16:21:00Z

cardinal/src/components/FileGridItem.tsx

+  const reportedWidth = item.iconWidth ? item.iconWidth / 2 : iconSize;
+  const reportedHeight = item.iconHeight ? item.iconHeight / 2 : iconSize;


Dividing icon dimensions by 2 without explanation. Lines 153-154 divide iconWidth and iconHeight by 2 with no comment explaining why. If this is related to retina/hi-DPI scaling, it should be documented. Consider using a named constant for the scale factor.

Suggested change

const reportedWidth = item.iconWidth ? item.iconWidth / 2 : iconSize;

const reportedHeight = item.iconHeight ? item.iconHeight / 2 : iconSize;

// Backend reports icon dimensions at 2x scale (physical pixels for retina/hi-DPI); convert to CSS pixels.

const BACKEND_ICON_SCALE = 2;

const reportedWidth = item.iconWidth ? item.iconWidth / BACKEND_ICON_SCALE : iconSize;

const reportedHeight = item.iconHeight ? item.iconHeight / BACKEND_ICON_SCALE : iconSize;

Copilot · 2025-12-27T16:21:00Z

cardinal/src/hooks/useIconViewport.ts

    requestIdRef.current += 1;
-    void invoke('update_icon_viewport', { id: requestIdRef.current, viewport });
-  }, []);
+    requestIdRef.current += 1;


Duplicate line detected. The line requestIdRef.current += 1; appears twice consecutively, which means the request ID is being incremented twice per update. This should only be incremented once.

Suggested change

requestIdRef.current += 1;

Copilot · 2025-12-27T16:21:00Z

cardinal/src-tauri/src/commands.rs

+#[tauri::command]
+pub async fn delete_paths(paths: Vec<String>) -> Result<(), String> {
+    for path in paths {
+        let p = PathBuf::from(path);
+        if p.is_dir() {
+            if let Err(e) = fs::remove_dir_all(&p) {
+                error!("Failed to delete directory {p:?}: {e}");
+                return Err(e.to_string());
+            }
+        } else if let Err(e) = fs::remove_file(&p) {
+            error!("Failed to delete file {p:?}: {e}");
+            return Err(e.to_string());
+        }


Missing error handling for file operations. The delete_paths function directly calls fs::remove_dir_all and fs::remove_file without validation. Consider adding checks to prevent deletion of system directories, ensure paths are within allowed boundaries, or confirm files exist before attempting deletion to provide better error messages.

Suggested change

#[tauri::command]

pub async fn delete_paths(paths: Vec<String>) -> Result<(), String> {

for path in paths {

let p = PathBuf::from(path);

if p.is_dir() {

if let Err(e) = fs::remove_dir_all(&p) {

error!("Failed to delete directory {p:?}: {e}");

return Err(e.to_string());

}

} else if let Err(e) = fs::remove_file(&p) {

error!("Failed to delete file {p:?}: {e}");

return Err(e.to_string());

}

/// Returns true if the given path is a protected filesystem location that should never be deleted.

/// This treats any filesystem root (e.g., `/` on Unix or `C:\` on Windows) as protected.

fn is_protected_path(path: &std::path::Path) -> bool {

// Only consider paths with a root component, and that have no parent (i.e., the root itself).

path.has_root() && path.parent().is_none()

}

#[tauri::command]

pub async fn delete_paths(paths: Vec<String>) -> Result<(), String> {

for path in paths {

let p = PathBuf::from(&path);

if !p.exists() {

warn!("Requested to delete non-existent path: {p:?}");

continue;

}

let canonical = match p.canonicalize() {

Ok(c) => c,

Err(e) => {

error!("Failed to canonicalize path {p:?}: {e}");

return Err(e.to_string());

}

};

if is_protected_path(&canonical) {

let msg = format!("Refusing to delete protected path: {canonical:?}");

error!("{msg}");

return Err(msg);

}

if canonical.is_dir() {

if let Err(e) = fs::remove_dir_all(&canonical) {

error!("Failed to delete directory {canonical:?}: {e}");

return Err(e.to_string());

}

} else if canonical.is_file() {

if let Err(e) = fs::remove_file(&canonical) {

error!("Failed to delete file {canonical:?}: {e}");

return Err(e.to_string());

}

} else {

// Path exists but is neither a regular file nor directory (e.g., symlink, special file).

// Let the caller know we didn't delete it.

let msg = format!("Refusing to delete unsupported path type: {canonical:?}");

warn!("{msg}");

return Err(msg);

}

Copilot · 2025-12-27T16:21:01Z

cardinal/src/App.tsx

              searchErrorMessage={searchErrorMessage}
              currentQuery={currentQuery}
-              virtualListRef={virtualListRef}
+              virtualListRef={virtualListRef as any}


The type cast as any bypasses TypeScript's type checking. While VirtualListHandle and VirtualGridHandle are compatible enough for the ref, using as any defeats the purpose of type safety. Consider using a union type or creating a common base interface that both handle types extend.

Copilot · 2025-12-27T16:21:01Z

cardinal/src/components/VirtualGrid.tsx

+  const handleWheel = useCallback(
+    (e: React.WheelEvent<HTMLDivElement>) => {
+      e.preventDefault();
+      updateScrollAndRange((prev) => prev + e.deltaY);
+      if (marqueeStart && marqueeEnd) {
+        setMarqueeEnd((prev) => (prev ? { ...prev, y: prev.y + e.deltaY } : null));
+      }
+    },
+    [updateScrollAndRange, marqueeStart, marqueeEnd],


The marquee selection should clear properly when scrolling completes. The current implementation updates marquee end position while scrolling but may leave stale state if the user scrolls without moving the mouse. Consider resetting marquee state when scroll ends without mouse movement.

Copilot · 2025-12-27T16:21:03Z

fs-icon/src/lib.rs

+        let width = {
+            let rep = NSBitmapImageRep::imageRepWithData(&png_data)?;
+            rep.pixelsWide() as f64
+        };
+        let height = {
+            let rep = NSBitmapImageRep::imageRepWithData(&png_data)?;
+            rep.pixelsHigh() as f64
+        };


Redundant NSBitmapImageRep creation. The code creates NSBitmapImageRep from png_data twice - once at line 108 to get width, and again at line 112 to get height. This is inefficient and should be done once, storing the result in a variable to query both dimensions.

Copilot · 2025-12-27T16:21:03Z

cardinal/src/App.css

+  /* Balanced top padding, 0 side padding, 8px bottom */
+  cursor: default;
+  border-radius: 8px;
+  border-radius: 8px;


Duplicate border-radius property. Line 1889 and 1890 both set border-radius to 8px. Remove the duplicate.

Suggested change

border-radius: 8px;

Copilot · 2025-12-27T16:21:04Z

cardinal/src/components/FileGridItem.tsx

@@ -0,0 +1,214 @@
+import React, { memo, useCallback, DragEvent, useRef } from 'react';


Unused import DragEvent.

Suggested change

import React, { memo, useCallback, DragEvent, useRef } from 'react';

import React, { memo, useCallback, useRef } from 'react';

Copilot · 2025-12-27T16:21:04Z

cardinal/src/App.tsx

 import { useQuickLook } from './hooks/useQuickLook';
 import { useSearchHistory } from './hooks/useSearchHistory';
-import { ROW_HEIGHT, OVERSCAN_ROW_COUNT } from './constants';
+import { ROW_HEIGHT, OVERSCAN_ROW_COUNT, DEFAULT_ICON_SIZE, type ViewMode } from './constants';


Unused import DEFAULT_ICON_SIZE.

Suggested change

import { ROW_HEIGHT, OVERSCAN_ROW_COUNT, DEFAULT_ICON_SIZE, type ViewMode } from './constants';

import { ROW_HEIGHT, OVERSCAN_ROW_COUNT, type ViewMode } from './constants';

Copilot · 2025-12-27T16:21:04Z

cardinal/src/components/VirtualGrid.tsx

+import type { SearchResultItem } from '../types/search';
+import type { SlabIndex } from '../types/slab';
+import { useIconViewport } from '../hooks/useIconViewport';
+import { CONTAINER_PADDING } from '../constants';


Unused import CONTAINER_PADDING.

Suggested change

import { CONTAINER_PADDING } from '../constants';

Added grid view mode

0ce8285

xgrommx force-pushed the grid-view-mode branch from 38c535b to 0ce8285 Compare December 25, 2025 18:52

ldm0 requested review from Copilot and ldm0 December 27, 2025 16:16

Copilot started reviewing on behalf of ldm0 December 27, 2025 16:17 View session

Copilot AI reviewed Dec 27, 2025

View reviewed changes

		const reportedWidth = item.iconWidth ? item.iconWidth / 2 : iconSize;
		const reportedHeight = item.iconHeight ? item.iconHeight / 2 : iconSize;

-  const reportedWidth = item.iconWidth ? item.iconWidth / 2 : iconSize;
-  const reportedHeight = item.iconHeight ? item.iconHeight / 2 : iconSize;
+  // Backend reports icon dimensions at 2x scale (physical pixels for retina/hi-DPI); convert to CSS pixels.
+  const BACKEND_ICON_SCALE = 2;
+  const reportedWidth = item.iconWidth ? item.iconWidth / BACKEND_ICON_SCALE : iconSize;
+  const reportedHeight = item.iconHeight ? item.iconHeight / BACKEND_ICON_SCALE : iconSize;

-#[tauri::command]
-pub async fn delete_paths(paths: Vec<String>) -> Result<(), String> {
-    for path in paths {
-        let p = PathBuf::from(path);
-        if p.is_dir() {
-            if let Err(e) = fs::remove_dir_all(&p) {
-                error!("Failed to delete directory {p:?}: {e}");
-                return Err(e.to_string());
-            }
-        } else if let Err(e) = fs::remove_file(&p) {
-            error!("Failed to delete file {p:?}: {e}");
-            return Err(e.to_string());
-        }
+/// Returns true if the given path is a protected filesystem location that should never be deleted.
+/// This treats any filesystem root (e.g., `/` on Unix or `C:\` on Windows) as protected.
+fn is_protected_path(path: &std::path::Path) -> bool {
+    // Only consider paths with a root component, and that have no parent (i.e., the root itself).
+    path.has_root() && path.parent().is_none()
+}
+#[tauri::command]
+pub async fn delete_paths(paths: Vec<String>) -> Result<(), String> {
+    for path in paths {
+        let p = PathBuf::from(&path);
+        if !p.exists() {
+            warn!("Requested to delete non-existent path: {p:?}");
+            continue;
+        }
+        let canonical = match p.canonicalize() {
+            Ok(c) => c,
+            Err(e) => {
+                error!("Failed to canonicalize path {p:?}: {e}");
+                return Err(e.to_string());
+            }
+        };
+        if is_protected_path(&canonical) {
+            let msg = format!("Refusing to delete protected path: {canonical:?}");
+            error!("{msg}");
+            return Err(msg);
+        }
+        if canonical.is_dir() {
+            if let Err(e) = fs::remove_dir_all(&canonical) {
+                error!("Failed to delete directory {canonical:?}: {e}");
+                return Err(e.to_string());
+            }
+        } else if canonical.is_file() {
+            if let Err(e) = fs::remove_file(&canonical) {
+                error!("Failed to delete file {canonical:?}: {e}");
+                return Err(e.to_string());
+            }
+        } else {
+            // Path exists but is neither a regular file nor directory (e.g., symlink, special file).
+            // Let the caller know we didn't delete it.
+            let msg = format!("Refusing to delete unsupported path type: {canonical:?}");
+            warn!("{msg}");
+            return Err(msg);
+        }

		@@ -0,0 +1,214 @@
		import React, { memo, useCallback, DragEvent, useRef } from 'react';

	import React, { memo, useCallback, DragEvent, useRef } from 'react';
	import React, { memo, useCallback, useRef } from 'react';

	import { ROW_HEIGHT, OVERSCAN_ROW_COUNT, DEFAULT_ICON_SIZE, type ViewMode } from './constants';
	import { ROW_HEIGHT, OVERSCAN_ROW_COUNT, type ViewMode } from './constants';

Grid view mode #99

Are you sure you want to change the base?

Grid view mode #99

Uh oh!

Conversation

xgrommx commented Dec 25, 2025

Uh oh!

xgrommx commented Dec 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 27, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

xgrommx commented Dec 25, 2025 •

edited

Loading