Skip to content
Merged
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
49 changes: 35 additions & 14 deletions src/components/data-grid/data-grid-cell-variants.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1364,12 +1364,12 @@ export function FileCell<TData>({
return file.type === type;
});
if (!isAccepted) {
return `File type not accepted. Accepted: ${accept}`;
return "File type not accepted";

Copilot AI Nov 20, 2025

Copy link

Choose a reason for hiding this comment

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

Usability regression: Removing the accepted file types from the error message (Accepted: ${accept}) means users no longer see which file types are accepted when their upload is rejected. Consider adding this information back to the toast notification, perhaps in the description field, or display accepted file types in the dropzone UI (lines 1758-1764) alongside the max size and max files information.

Suggested change
return "File type not accepted";
return `File type not accepted. Accepted: ${acceptedTypes.join(", ")}`;

Copilot uses AI. Check for mistakes.
}
}
return null;
},
[maxFileSize, acceptedTypes, accept],
[maxFileSize, acceptedTypes],
);

const addFiles = React.useCallback(
Expand All @@ -1378,19 +1378,22 @@ export function FileCell<TData>({

// Check max files limit
if (maxFiles && files.length + newFiles.length > maxFiles) {
setError(`Maximum ${maxFiles} files allowed`);
const errorMessage = `Maximum ${maxFiles} files allowed`;
setError(errorMessage);
toast(errorMessage);

Copilot AI Nov 20, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] Inconsistent error notification: When max files limit is exceeded, only a toast is shown (line 1383), but for file validation errors, both a toast with description is shown (lines 1422-1429). Consider adding a description to the max files toast for consistency, such as { description: "Please remove some files before adding more" } to provide users with actionable guidance.

Suggested change
toast(errorMessage);
toast(errorMessage, {
description: "Please remove some files before adding more",
});

Copilot uses AI. Check for mistakes.
setTimeout(() => {
setError(null);
}, 2000);
Comment on lines +1384 to +1386

Copilot AI Nov 20, 2025

Copy link

Choose a reason for hiding this comment

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

Potential memory leak: The setTimeout is not cleaned up if the component unmounts before the timeout fires. Store the timeout ID and clean it up in a useEffect cleanup function to prevent setting state on an unmounted component.

Copilot uses AI. Check for mistakes.
return;
}

const validFiles: FileCellData[] = [];
let firstError: string | null = null;
const rejectedFiles: Array<{ name: string; reason: string }> = [];

for (const file of newFiles) {
const validationError = validateFile(file);
if (validationError) {
if (!firstError) {
firstError = validationError;
}
rejectedFiles.push({ name: file.name, reason: validationError });
continue;
}

Expand All @@ -1405,8 +1408,30 @@ export function FileCell<TData>({
validFiles.push(fileData);
}

if (firstError) {
setError(firstError);
if (rejectedFiles.length > 0) {
const firstError = rejectedFiles[0];
if (firstError) {
setError(firstError.reason);

const truncatedName =
firstError.name.length > 20
? `${firstError.name.slice(0, 20)}...`
: firstError.name;

if (rejectedFiles.length === 1) {
toast(firstError.reason, {
description: `"${truncatedName}" has been rejected`,
});
} else {
toast(firstError.reason, {
description: `"${truncatedName}" and ${rejectedFiles.length - 1} more rejected`,
});
}

setTimeout(() => {
setError(null);
}, 2000);
Comment on lines +1431 to +1433

Copilot AI Nov 20, 2025

Copy link

Choose a reason for hiding this comment

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

Accessibility concern: The error state is used for aria-invalid (line 1736), but it's automatically cleared after 2 seconds via setTimeout. This means screen readers and assistive technologies will lose the invalid state even though the user hasn't corrected the issue. Consider either not auto-clearing the error state, or only clearing it when the user takes a corrective action (e.g., removing files, closing the dialog).

Suggested change
setTimeout(() => {
setError(null);
}, 2000);

Copilot uses AI. Check for mistakes.
Comment on lines +1431 to +1433

Copilot AI Nov 20, 2025

Copy link

Choose a reason for hiding this comment

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

Potential memory leak: The setTimeout is not cleaned up if the component unmounts before the timeout fires. Store the timeout ID and clean it up in a useEffect cleanup function to prevent setting state on an unmounted component.

Copilot uses AI. Check for mistakes.
}
}

if (validFiles.length > 0) {
Expand Down Expand Up @@ -1582,6 +1607,7 @@ export function FileCell<TData>({
const onOpenChange = React.useCallback(
(isOpen: boolean) => {
if (isOpen) {
setError(null);
meta?.onCellEditingStart?.(rowIndex, columnId);
} else {
setError(null);
Expand Down Expand Up @@ -1747,11 +1773,6 @@ export function FileCell<TData>({
ref={fileInputRef}
onChange={onFileInputChange}
/>
{error && (
<div className="rounded-md bg-destructive/10 px-3 py-2 text-destructive text-xs">
{error}
</div>
)}
{files.length > 0 && (
<div className="flex flex-col gap-2">
<div className="flex items-center justify-between">
Expand Down
Loading