Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 38 additions & 14 deletions apps/viewer/src/components/viewer/CommandPalette.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ import { SCRIPT_TEMPLATES } from '@/lib/scripts/templates';
import { exportGlbFromGeometry } from '@/lib/export/glb';
import { exportCsvFromBytes } from '@/lib/export/csv';
import { downloadFile } from '@/lib/export/download';
import { getRecentFiles, formatFileSize, getCachedFile } from '@/lib/recent-files';
import { getRecentFiles, formatFileSize, getCachedFile, getCachedFileNames } from '@/lib/recent-files';
import type { RecentFileEntry } from '@/lib/recent-files';
import { closeActiveAnalysisExtension } from '@/services/analysis-extensions';
import { describeRunCommandError } from '@/services/extensions/runtime-errors';
Expand Down Expand Up @@ -120,6 +120,13 @@ interface Command {
shortcut?: string;
detail?: string; // subtle secondary text (e.g. file size)
action: () => void;
/**
* Run the action synchronously inside the click handler instead of deferring
* to the next animation frame. Required for actions that open a file dialog:
* Chrome only honours `input.click()` / `showOpenFilePicker()` while transient
* user activation is live, which a `requestAnimationFrame` hop would discard.
*/
immediate?: boolean;
}

interface FlatItem {
Expand Down Expand Up @@ -246,6 +253,9 @@ export function CommandPalette({ open, onOpenChange }: CommandPaletteProps) {
const inputRef = useRef<HTMLInputElement>(null);
const listRef = useRef<HTMLDivElement>(null);
const navigatedByKeyboard = useRef(false);
// Names currently in the blob cache, refreshed each open. Lets recent-file
// clicks decide hit/miss without an async gap that would void user activation.
const cachedNamesRef = useRef<Set<string>>(new Set());

const { execute } = useSandbox();
const extensionCommands = useSlotContributions<CommandContribution>('commandPalette');
Expand All @@ -255,6 +265,7 @@ export function CommandPalette({ open, onOpenChange }: CommandPaletteProps) {
if (open) {
setRecentIds(getRecentIds());
setRecentFiles(getRecentFiles());
void getCachedFileNames().then((names) => { cachedNamesRef.current = new Set(names); });
setQuery('');
requestAnimationFrame(() => inputRef.current?.focus());
}
Expand All @@ -265,11 +276,15 @@ export function CommandPalette({ open, onOpenChange }: CommandPaletteProps) {
const c: Command[] = [];

// ── File ──
// `immediate` so the open action runs inside the click gesture: opening a
// file dialog needs live user activation, which a rAF hop would discard.
// The actual picker is driven by MainToolbar's handleOpenClick (via the
// `ifc-lite:open-files` event) so palette opens capture a live handle too.
c.push(
{ id: 'file:open', label: 'Open File', keywords: 'ifc ifcx glb load model browse', category: 'File', icon: FolderOpen,
immediate: true,
action: () => {
const input = document.getElementById('file-input-open') as HTMLInputElement | null;
if (input) input.click();
window.dispatchEvent(new CustomEvent('ifc-lite:open-files'));
} },
);
for (const rf of recentFiles) {
Expand All @@ -279,17 +294,20 @@ export function CommandPalette({ open, onOpenChange }: CommandPaletteProps) {
keywords: `recent open ${formatFileSize(rf.size)}`,
category: 'File', icon: Clock,
detail: formatFileSize(rf.size),
immediate: true,
action: () => {
// Try loading from IndexedDB blob cache → dispatches to MainToolbar's loadFile
getCachedFile(rf).then(file => {
if (file) {
window.dispatchEvent(new CustomEvent('ifc-lite:load-file', { detail: file }));
} else {
// Cache miss — fall back to file picker
const input = document.getElementById('file-input-open') as HTMLInputElement | null;
if (input) input.click();
}
});
// Cached (decided synchronously from the pre-loaded key set): load the
// blob — dispatching a load event needs no user activation.
if (cachedNamesRef.current.has(fileName)) {
void getCachedFile(rf).then(file => {
if (file) window.dispatchEvent(new CustomEvent('ifc-lite:load-file', { detail: file }));
else window.dispatchEvent(new CustomEvent('ifc-lite:open-files'));
});
} else {
// Not cached — re-pick. Synchronous within the gesture so the dialog
// actually opens on Chrome.
window.dispatchEvent(new CustomEvent('ifc-lite:open-files'));
}
},
});
}
Expand Down Expand Up @@ -634,7 +652,13 @@ export function CommandPalette({ open, onOpenChange }: CommandPaletteProps) {
const runCommand = useCallback((cmd: Command) => {
onOpenChange(false);
recordUsage(cmd.id);
requestAnimationFrame(() => cmd.action());
// File-dialog actions must run while user activation is still live; deferring
// them to a frame later voids it and Chrome silently ignores the dialog.
if (cmd.immediate) {
cmd.action();
} else {
requestAnimationFrame(() => cmd.action());
}
}, [onOpenChange]);

const handleKeyDown = useCallback((e: React.KeyboardEvent) => {
Expand Down
207 changes: 183 additions & 24 deletions apps/viewer/src/components/viewer/MainToolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import {
Redo2,
Boxes,
Shapes,
RefreshCw,
} from 'lucide-react';
import { Button } from '@/components/ui/button';
import { Switch } from '@/components/ui/switch';
Expand All @@ -69,7 +70,7 @@ import {
DropdownMenuSubContent,
} from '@/components/ui/dropdown-menu';
import { Progress } from '@/components/ui/progress';
import { useViewerStore, isIfcxDataStore } from '@/store';
import { useViewerStore, isIfcxDataStore, type FederatedModel } from '@/store';
import { goHomeFromStore, resetVisibilityForHomeFromStore } from '@/store/homeView';
import { executeBasketIsolate } from '@/store/basket/basketCommands';
import { useIfc } from '@/hooks/useIfc';
Expand All @@ -85,6 +86,11 @@ import { DataConnector } from './DataConnector';
import { ExportChangesButton } from './ExportChangesButton';
import { SearchInline } from './SearchInline';
import { recordRecentFiles, cacheFileBlobs } from '@/lib/recent-files';
import {
supportsFileSystemAccess,
openIfcFilesWithHandles,
readFreshFile,
} from '@/services/file-system-access';
import { ThemeSwitch } from './ThemeSwitch';
import { ExtensionToolbarSlot } from '@/components/extensions/ExtensionToolbarSlot';
import { toast } from '@/components/ui/toast';
Expand Down Expand Up @@ -282,6 +288,19 @@ function ActionButton({ icon: Icon, label, onClick, shortcut, disabled }: Action
}
// #endregion

/** Extensions the viewer can ingest (IFC / IFCX / GLB / point clouds). */
function isSupportedModelFile(f: File): boolean {
const n = f.name.toLowerCase();
return n.endsWith('.ifc') || n.endsWith('.ifcx') || n.endsWith('.glb')
|| n.endsWith('.las') || n.endsWith('.laz') || n.endsWith('.ply') || n.endsWith('.pcd')
|| n.endsWith('.e57') || n.endsWith('.pts') || n.endsWith('.xyz');
}

/** Case-insensitive IFCX check (filenames are accepted case-insensitively). */
function isIfcxModelFile(f: File): boolean {
return f.name.toLowerCase().endsWith('.ifcx');
}

interface MainToolbarProps {
onShowShortcuts?: () => void;
}
Expand Down Expand Up @@ -428,11 +447,8 @@ export function MainToolbar({ onShowShortcuts }: MainToolbarProps = {} as MainTo
const files = e.target.files;
if (!files || files.length === 0) return;

// Filter to supported files (IFC, IFCX, GLB)
const supportedFiles = Array.from(files).filter(
f => f.name.endsWith('.ifc') || f.name.endsWith('.ifcx') || f.name.endsWith('.glb')
|| f.name.toLowerCase().endsWith('.las') || f.name.toLowerCase().endsWith('.laz') || f.name.toLowerCase().endsWith('.ply') || f.name.toLowerCase().endsWith('.pcd') || f.name.toLowerCase().endsWith('.e57') || f.name.toLowerCase().endsWith('.pts') || f.name.toLowerCase().endsWith('.xyz')
);
// Filter to supported files (IFC, IFCX, GLB, point clouds)
const supportedFiles = Array.from(files).filter(isSupportedModelFile);
Comment on lines +450 to +451

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Normalize IFCX detection after accepting case-insensitive filenames.

isSupportedModelFile accepts MODEL.IFCX, but the raw .endsWith('.ifcx') checks route it as non-IFCX, skipping IFCX composition/overlay behavior.

Suggested fix
 function isSupportedModelFile(f: File): boolean {
   const n = f.name.toLowerCase();
   return n.endsWith('.ifc') || n.endsWith('.ifcx') || n.endsWith('.glb')
     || n.endsWith('.las') || n.endsWith('.laz') || n.endsWith('.ply') || n.endsWith('.pcd')
     || n.endsWith('.e57') || n.endsWith('.pts') || n.endsWith('.xyz');
 }
+
+function isIfcxModelFile(f: File): boolean {
+  return f.name.toLowerCase().endsWith('.ifcx');
+}
 
-    const newFilesAreIfcx = supportedFiles.every(f => f.name.endsWith('.ifcx'));
+    const newFilesAreIfcx = supportedFiles.every(isIfcxModelFile);
 
-      const allIfcx = files.every(f => f.name.endsWith('.ifcx'));
+      const allIfcx = files.every(isIfcxModelFile);

Also applies to: 487-488, 548-548

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/viewer/src/components/viewer/MainToolbar.tsx` around lines 445 - 446,
The IFCX handling in MainToolbar is inconsistent because isSupportedModelFile
now accepts case-insensitive filenames, but later logic still checks raw string
suffixes like .endsWith('.ifcx'), causing MODEL.IFCX to skip IFCX-specific
composition and overlay behavior. Update the downstream IFCX detection in
MainToolbar (including the branches around the supportedFiles flow and the later
composition/overlay checks) to normalize filenames before testing suffixes, and
use a single shared IFCX check so MODEL.IFCX is treated the same regardless of
case.


if (supportedFiles.length === 0) return;

Expand Down Expand Up @@ -465,38 +481,161 @@ export function MainToolbar({ onShowShortcuts }: MainToolbarProps = {} as MainTo
e.target.value = '';
}, [loadFile, loadFilesSequentially, loadFederatedIfcx, resetViewerState, clearAllModels]);

const handleAddModelSelect = useCallback((e: React.ChangeEvent<HTMLInputElement>) => {
const files = e.target.files;
if (!files || files.length === 0) return;

// Filter to supported files (IFC, IFCX, GLB)
const supportedFiles = Array.from(files).filter(
f => f.name.endsWith('.ifc') || f.name.endsWith('.ifcx') || f.name.endsWith('.glb')
|| f.name.toLowerCase().endsWith('.las') || f.name.toLowerCase().endsWith('.laz') || f.name.toLowerCase().endsWith('.ply') || f.name.toLowerCase().endsWith('.pcd') || f.name.toLowerCase().endsWith('.e57') || f.name.toLowerCase().endsWith('.pts') || f.name.toLowerCase().endsWith('.xyz')
);

// Shared Add-Model routing. `handles` is positionally aligned with
// `supportedFiles`, carrying a live FS Access handle per file (Chromium) so
// each added model stays part of a refreshable federation.
const addSupportedFiles = useCallback((
supportedFiles: File[],
handles?: (FileSystemFileHandle | undefined)[],
) => {
if (supportedFiles.length === 0) return;

// Check if adding IFCX files
const newFilesAreIfcx = supportedFiles.every(f => f.name.endsWith('.ifcx'));
const newFilesAreIfcx = supportedFiles.every(isIfcxModelFile);
const existingIsIfcx = isIfcxDataStore(ifcDataStore);

if (newFilesAreIfcx && existingIsIfcx) {
// Adding IFCX overlay(s) to existing IFCX model - re-compose with new layers
console.log(`[MainToolbar] Adding ${supportedFiles.length} IFCX overlay(s) to existing IFCX model - re-composing`);
addIfcxOverlays(supportedFiles);
void addIfcxOverlays(supportedFiles);
} else if (newFilesAreIfcx && !existingIsIfcx && ifcDataStore) {
// User trying to add IFCX to IFC4 model - won't work
console.warn('[MainToolbar] Cannot add IFCX files to non-IFCX model');
alert(`IFCX overlay files cannot be added to IFC4 models.\n\nPlease load IFCX files separately.`);
} else {
// Standard case - add as independent models (IFC4, GLB, or mixed)
loadFilesSequentially(supportedFiles);
void loadFilesSequentially(supportedFiles, handles);
}
}, [loadFilesSequentially, addIfcxOverlays, ifcDataStore]);

const handleAddModelSelect = useCallback((e: React.ChangeEvent<HTMLInputElement>) => {
const files = e.target.files;
if (!files || files.length === 0) return;
// <input> yields no live handle, so models added this way aren't refreshable.
const supportedFiles = Array.from(files).filter(isSupportedModelFile);
addSupportedFiles(supportedFiles);
// Reset input so same files can be selected again
e.target.value = '';
}, [loadFilesSequentially, addIfcxOverlays, ifcDataStore]);
}, [addSupportedFiles]);

// Preferred Add-Model path: the picker captures a handle per file so the
// resulting federation can be refreshed. Falls back to the hidden <input>.
const handleAddModelClick = useCallback(async () => {
if (!supportsFileSystemAccess()) {
addModelInputRef.current?.click();
return;
}
const opened = await openIfcFilesWithHandles();
if (!opened) return;
const supported = opened.filter(o => isSupportedModelFile(o.file));
addSupportedFiles(supported.map(o => o.file), supported.map(o => o.handle));
}, [addSupportedFiles]);

// Open via the File System Access API when available (Chromium) so we capture
// a live FileSystemFileHandle for each file — that handle is what lets the
// Refresh button re-read the same file from disk later (issue #1345). Browsers
// without the API fall back to the hidden <input type="file">.
const handleOpenClick = useCallback(async () => {
if (!supportsFileSystemAccess()) {
fileInputRef.current?.click();
return;
}
const picked = await openIfcFilesWithHandles();
if (!picked) return; // cancelled, unavailable, or picker failed
// The picker keeps an "all files" option, so drop anything unsupported
// before it reaches the load pipeline (matches the <input> + Add Model paths).
const opened = picked.filter(o => isSupportedModelFile(o.file));
if (opened.length === 0) return;

const files = opened.map(o => o.file);
recordRecentFiles(files.map(f => ({ name: f.name, size: f.size })));
void cacheFileBlobs(files);

if (opened.length === 1) {
// Single model: keep the handle so Refresh can re-read it from disk.
void loadFile(opened[0].file, { kind: 'primary' }, { sourceHandle: opened[0].handle });
} else {
// Multiple files mirror handleFileSelect's branching.
const allIfcx = files.every(isIfcxModelFile);
resetViewerState();
clearAllModels();
if (allIfcx) {
// IFCX layers compose into one shared store — no per-file handle.
void loadFederatedIfcx(files);
} else {
// Carry each file's handle so the whole federation stays refreshable.
void loadFilesSequentially(files, opened.map(o => o.handle));
}
}
}, [loadFile, loadFilesSequentially, loadFederatedIfcx, resetViewerState, clearAllModels]);

// Refresh re-reads files from disk and re-parses them. Offered when EVERY
// loaded model has a live FS Access handle (a single model, or a federation
// fully opened via the picker/drag this session). Drag-drop on non-Chromium,
// <input type="file">, cache-restored, and IFCX-composed models have no
// handle, so a mixed session hides the button rather than risk dropping the
// handle-less models during the rebuild.
const canRefresh = useMemo(() => {
if (loading || models.size === 0) return false;
return Array.from(models.values()).every(m => m.sourceHandle);
}, [models, loading]);

const handleRefresh = useCallback(async () => {
const targets = (Array.from(useViewerStore.getState().models.values()) as FederatedModel[])
.filter((m): m is FederatedModel & { sourceHandle: FileSystemFileHandle } => Boolean(m.sourceHandle))
.sort((a, b) => (a.loadedAt ?? 0) - (b.loadedAt ?? 0));
if (targets.length === 0) return;

// Re-read every handle BEFORE clearing anything, so a failed read never
// leaves the viewer empty.
const reads = await Promise.all(
targets.map(async (m) => ({ model: m, fresh: await readFreshFile(m.sourceHandle) })),
);
const ok = reads.filter((r) => r.fresh) as { model: typeof targets[number]; fresh: File }[];
const failedNames = reads.filter((r) => !r.fresh).map((r) => `"${r.model.name}"`);

if (ok.length === 0) {
toast.error(`Couldn't re-read ${failedNames.join(', ')}. Files may have moved, been deleted, or access was denied.`);
return;
}

recordRecentFiles(ok.map((r) => ({ name: r.fresh.name, size: r.fresh.size })));
void cacheFileBlobs(ok.map((r) => r.fresh));

if (targets.length === 1) {
// Await so the success toast only fires once the reload has completed.
await loadFile(ok[0].fresh, { kind: 'primary' }, { sourceHandle: ok[0].model.sourceHandle });
} else {
// Rebuild the federation from fresh bytes, preserving id + order + state.
clearAllModels();
for (const r of ok) {
const reloadedId = await addModel(r.fresh, {
name: r.model.name,
modelId: r.model.id,
loadedAt: r.model.loadedAt,
visible: r.model.visible,
collapsed: r.model.collapsed,
sourceHandle: r.model.sourceHandle,
});
if (reloadedId && r.model.visible === false) {
useViewerStore.getState().setModelVisibility(r.model.id, false);
}
}
}

if (failedNames.length > 0) {
toast.error(`Refreshed ${ok.length}; couldn't re-read ${failedNames.join(', ')}.`);
} else {
toast.success(ok.length === 1 ? `Refreshed "${ok[0].fresh.name}"` : `Refreshed ${ok.length} models`);
}
Comment on lines +592 to +628

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Keep Refresh atomic and report success only after reload completes.

On partial read failure, the multi-model branch clears the scene and reloads only ok, dropping failed models. The single-model branch also fires a success toast before loadFile finishes.

Suggested fix
-    const ok = reads.filter((r) => r.fresh) as { model: typeof targets[number]; fresh: File }[];
     const failedNames = reads.filter((r) => !r.fresh).map((r) => `"${r.model.name}"`);
 
-    if (ok.length === 0) {
+    if (failedNames.length > 0) {
       toast.error(`Couldn't re-read ${failedNames.join(', ')}. Files may have moved, been deleted, or access was denied.`);
       return;
     }
+    const ok = reads as { model: typeof targets[number]; fresh: File }[];
 
     recordRecentFiles(ok.map((r) => ({ name: r.fresh.name, size: r.fresh.size })));
     void cacheFileBlobs(ok.map((r) => r.fresh));
 
-    if (targets.length === 1) {
-      void loadFile(ok[0].fresh, { kind: 'primary' }, { sourceHandle: ok[0].model.sourceHandle });
-    } else {
-      // Rebuild the federation from fresh bytes, preserving id + order + state.
-      clearAllModels();
-      for (const r of ok) {
-        const reloadedId = await addModel(r.fresh, {
-          name: r.model.name,
-          modelId: r.model.id,
-          loadedAt: r.model.loadedAt,
-          visible: r.model.visible,
-          collapsed: r.model.collapsed,
-          sourceHandle: r.model.sourceHandle,
-        });
-        if (reloadedId && r.model.visible === false) {
-          useViewerStore.getState().setModelVisibility(r.model.id, false);
+    try {
+      if (targets.length === 1) {
+        await loadFile(ok[0].fresh, { kind: 'primary' }, { sourceHandle: ok[0].model.sourceHandle });
+      } else {
+        // Rebuild the federation from fresh bytes, preserving id + order + state.
+        clearAllModels();
+        for (const r of ok) {
+          const reloadedId = await addModel(r.fresh, {
+            name: r.model.name,
+            modelId: r.model.id,
+            loadedAt: r.model.loadedAt,
+            visible: r.model.visible,
+            collapsed: r.model.collapsed,
+            sourceHandle: r.model.sourceHandle,
+          });
+          if (reloadedId && r.model.visible === false) {
+            useViewerStore.getState().setModelVisibility(r.model.id, false);
+          }
         }
       }
+    } catch (err) {
+      console.error('[MainToolbar] Refresh reload failed', err);
+      toast.error(`Refresh failed: ${err instanceof Error ? err.message : 'Unknown error'}`);
+      return;
     }
 
-    if (failedNames.length > 0) {
-      toast.error(`Refreshed ${ok.length}; couldn't re-read ${failedNames.join(', ')}.`);
-    } else {
-      toast.success(ok.length === 1 ? `Refreshed "${ok[0].fresh.name}"` : `Refreshed ${ok.length} models`);
-    }
+    toast.success(ok.length === 1 ? `Refreshed "${ok[0].fresh.name}"` : `Refreshed ${ok.length} models`);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const ok = reads.filter((r) => r.fresh) as { model: typeof targets[number]; fresh: File }[];
const failedNames = reads.filter((r) => !r.fresh).map((r) => `"${r.model.name}"`);
if (ok.length === 0) {
toast.error(`Couldn't re-read ${failedNames.join(', ')}. Files may have moved, been deleted, or access was denied.`);
return;
}
recordRecentFiles(ok.map((r) => ({ name: r.fresh.name, size: r.fresh.size })));
void cacheFileBlobs(ok.map((r) => r.fresh));
if (targets.length === 1) {
void loadFile(ok[0].fresh, { kind: 'primary' }, { sourceHandle: ok[0].model.sourceHandle });
} else {
// Rebuild the federation from fresh bytes, preserving id + order + state.
clearAllModels();
for (const r of ok) {
const reloadedId = await addModel(r.fresh, {
name: r.model.name,
modelId: r.model.id,
loadedAt: r.model.loadedAt,
visible: r.model.visible,
collapsed: r.model.collapsed,
sourceHandle: r.model.sourceHandle,
});
if (reloadedId && r.model.visible === false) {
useViewerStore.getState().setModelVisibility(r.model.id, false);
}
}
}
if (failedNames.length > 0) {
toast.error(`Refreshed ${ok.length}; couldn't re-read ${failedNames.join(', ')}.`);
} else {
toast.success(ok.length === 1 ? `Refreshed "${ok[0].fresh.name}"` : `Refreshed ${ok.length} models`);
}
const failedNames = reads.filter((r) => !r.fresh).map((r) => `"${r.model.name}"`);
if (failedNames.length > 0) {
toast.error(`Couldn't re-read ${failedNames.join(', ')}. Files may have moved, been deleted, or access was denied.`);
return;
}
const ok = reads as { model: typeof targets[number]; fresh: File }[];
recordRecentFiles(ok.map((r) => ({ name: r.fresh.name, size: r.fresh.size })));
void cacheFileBlobs(ok.map((r) => r.fresh));
try {
if (targets.length === 1) {
await loadFile(ok[0].fresh, { kind: 'primary' }, { sourceHandle: ok[0].model.sourceHandle });
} else {
// Rebuild the federation from fresh bytes, preserving id + order + state.
clearAllModels();
for (const r of ok) {
const reloadedId = await addModel(r.fresh, {
name: r.model.name,
modelId: r.model.id,
loadedAt: r.model.loadedAt,
visible: r.model.visible,
collapsed: r.model.collapsed,
sourceHandle: r.model.sourceHandle,
});
if (reloadedId && r.model.visible === false) {
useViewerStore.getState().setModelVisibility(r.model.id, false);
}
}
}
} catch (err) {
console.error('[MainToolbar] Refresh reload failed', err);
toast.error(`Refresh failed: ${err instanceof Error ? err.message : 'Unknown error'}`);
return;
}
toast.success(ok.length === 1 ? `Refreshed "${ok[0].fresh.name}"` : `Refreshed ${ok.length} models`);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/viewer/src/components/viewer/MainToolbar.tsx` around lines 583 - 618,
The refresh flow in MainToolbar’s reload handler is not atomic: the multi-model
path clears the viewer and reloads only the fresh entries, which drops failed
models, and the single-model path shows success before loadFile finishes. Update
the reload logic so the existing scene/state is only replaced after all reload
operations complete successfully, and keep failed models untouched when any read
fails. Use the existing symbols ok, failedNames, loadFile, addModel, and
clearAllModels to locate and adjust the refresh branch, and move the success
toast to fire only after the async reload work has fully completed.

}, [loadFile, addModel, clearAllModels]);

// The command palette dispatches this (synchronously, inside the click) so the
// toolbar's handle-capturing open path runs while user activation is still
// live — required for the file dialog to actually open on Chrome.
useEffect(() => {
const handler = () => { void handleOpenClick(); };
window.addEventListener('ifc-lite:open-files', handler);
return () => window.removeEventListener('ifc-lite:open-files', handler);
}, [handleOpenClick]);

const hasSelection = selectedEntityId !== null;
// Selection chip uses the multi-select size when present; falls back
Expand Down Expand Up @@ -788,7 +927,7 @@ export function MainToolbar({ onShowShortcuts }: MainToolbarProps = {} as MainTo
onClick={(e) => {
// Blur button to close tooltip before opening file dialog
(e.currentTarget as HTMLButtonElement).blur();
fileInputRef.current?.click();
void handleOpenClick();
}}
disabled={loading}
>
Expand All @@ -802,6 +941,26 @@ export function MainToolbar({ onShowShortcuts }: MainToolbarProps = {} as MainTo
<TooltipContent>Open IFC File</TooltipContent>
</Tooltip>

{canRefresh && (
<Tooltip>
<TooltipTrigger asChild>
<Button
variant="ghost"
size="icon-sm"
onClick={(e) => {
(e.currentTarget as HTMLButtonElement).blur();
void handleRefresh();
}}
disabled={loading}
aria-label={models.size > 1 ? 'Refresh models from disk' : 'Refresh model from disk'}
>
<RefreshCw className="h-4 w-4" />
</Button>
Comment on lines +947 to +958

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Add an accessible name to the icon-only Refresh button.

The new button has only an SVG icon and tooltip; give it an aria-label so assistive tech can identify the action.

Suggested fix
             <Button
               variant="ghost"
               size="icon-sm"
+              aria-label={models.size > 1 ? 'Refresh models from disk' : 'Refresh model from disk'}
               onClick={(e) => {
                 (e.currentTarget as HTMLButtonElement).blur();
                 void handleRefresh();
               }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Button
variant="ghost"
size="icon-sm"
onClick={(e) => {
(e.currentTarget as HTMLButtonElement).blur();
void handleRefresh();
}}
disabled={loading}
>
<RefreshCw className="h-4 w-4" />
</Button>
<Button
variant="ghost"
size="icon-sm"
aria-label={models.size > 1 ? 'Refresh models from disk' : 'Refresh model from disk'}
onClick={(e) => {
(e.currentTarget as HTMLButtonElement).blur();
void handleRefresh();
}}
disabled={loading}
>
<RefreshCw className="h-4 w-4" />
</Button>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/viewer/src/components/viewer/MainToolbar.tsx` around lines 937 - 947,
The icon-only Refresh button in MainToolbar lacks an accessible name, so add an
aria-label directly on the Button used with RefreshCw and handleRefresh. Keep
the tooltip if desired, but ensure assistive tech can identify the action from
the button itself by giving it a clear label such as “Refresh”.

</TooltipTrigger>
<TooltipContent>{models.size > 1 ? 'Refresh models from disk' : 'Refresh model from disk'}</TooltipContent>
</Tooltip>
)}

{/* Add Model button - only shown when models are loaded */}
{hasModelsLoaded && (
<Tooltip>
Expand All @@ -811,7 +970,7 @@ export function MainToolbar({ onShowShortcuts }: MainToolbarProps = {} as MainTo
size="icon-sm"
onClick={(e) => {
(e.currentTarget as HTMLButtonElement).blur();
addModelInputRef.current?.click();
void handleAddModelClick();
}}
disabled={loading}
className="text-[#9ece6a] hover:text-[#9ece6a] hover:bg-[#9ece6a]/10"
Expand Down
Loading
Loading