fix(platform): polish logo/favicon upload button hover and feedback (#1961)

services/platform/app/features/settings/branding/components/image-upload-field.stories.tsx

@@ -50,6 +50,22 @@ export const EmptyWithLabel: Story = {
   },
 };
 
+export const WithImage: Story = {
+  args: {
+    currentUrl:
+      'data:image/svg+xml,%3Csvg%20xmlns%3D%22http%3A//www.w3.org/2000/svg%22%20viewBox%3D%220%200%2048%2048%22%3E%3Crect%20width%3D%2248%22%20height%3D%2248%22%20rx%3D%228%22%20fill%3D%22%236366f1%22/%3E%3C/svg%3E',
+    ariaLabel: 'Upload logo',
+  },
+  parameters: {
+    docs: {
+      description: {
+        story:
+          'With an uploaded image. Hover or focus the control to reveal the replace overlay; a remove button sits in the corner.',
+      },
+    },
+  },
+};
+
 export const SmallSize: Story = {
   args: {
     size: 'sm',

services/platform/app/features/settings/branding/components/image-upload-field.test.tsx

@@ -0,0 +1,145 @@
+// @vitest-environment jsdom
+import '@testing-library/jest-dom/vitest';
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+
+import { checkAccessibility } from '@/tests/utils/a11y';
+
+// Mock the save-image mutation so the upload path resolves without a backend.
+// `mockSaveImage` is reassigned per-test (e.g. to a deferred promise) to drive
+// the in-flight `isUploading` state.
+let mockSaveImage = vi.fn().mockResolvedValue({ filename: 'logo.png' });
+vi.mock('../hooks/mutations', () => ({
+  useSaveImage: () => ({ mutateAsync: (args: unknown) => mockSaveImage(args) }),
+}));
+
+// The Next.js Image wrapper is only reached when an existing `currentUrl` is
+// shown; render a plain <img> so jsdom has nothing to resolve.
+vi.mock('@/app/components/ui/data-display/image', () => ({
+  Image: (props: Record<string, unknown>) => (
+    <img src={props.src as string} alt={props.alt as string} />
+  ),
+}));
+
+import { ImageUploadField } from './image-upload-field';
+
+const baseProps = {
+  organizationId: 'org_demo',
+  imageType: 'logo' as const,
+  ariaLabel: 'Upload logo',
+};
+
+function makeFile(name: string, type: string) {
+  return new File(['fake-image-bytes'], name, { type });
+}
+
+function dropFile(element: Element, file: File) {
+  fireEvent.drop(element, { dataTransfer: { files: [file] } });
+}
+
+beforeEach(() => {
+  mockSaveImage = vi.fn().mockResolvedValue({ filename: 'logo.png' });
+  // jsdom does not implement object-URL APIs the component calls on upload.
+  global.URL.createObjectURL = vi.fn(() => 'blob:preview');
+  global.URL.revokeObjectURL = vi.fn();
+});
+
+afterEach(() => {
+  vi.clearAllMocks();
+});
+
+describe('ImageUploadField', () => {
+  it('renders an accessible upload control', () => {
+    render(<ImageUploadField {...baseProps} onUpload={vi.fn()} />);
+    expect(
+      screen.getByRole('button', { name: 'Upload logo' }),
+    ).toBeInTheDocument();
+  });
+
+  // Happy path: a valid image dropped onto the control runs the upload and
+  // reports the saved filename back to the parent.
+  it('uploads a valid image dropped onto the control', async () => {
+    const onUpload = vi.fn();
+    render(<ImageUploadField {...baseProps} onUpload={onUpload} />);
+
+    dropFile(
+      screen.getByRole('button', { name: 'Upload logo' }),
+      makeFile('logo.png', 'image/png'),
+    );
+
+    await waitFor(() => expect(onUpload).toHaveBeenCalledTimes(1));
+    expect(mockSaveImage).toHaveBeenCalledTimes(1);
+    expect(mockSaveImage).toHaveBeenCalledWith(
+      expect.objectContaining({
+        organizationId: 'org_demo',
+        type: 'logo',
+        mimeType: 'image/png',
+      }),
+    );
+    expect(onUpload).toHaveBeenCalledWith('logo.png', expect.any(File));
+  });
+
+  // Edge: a second drop while an upload is in flight must be ignored so the
+  // same control cannot fire two concurrent uploads.
+  it('ignores a drop while an upload is already in flight', async () => {
+    // Keep the first upload pending so `isUploading` stays true for the second.
+    let resolveUpload: (value: { filename: string }) => void = () => {};
+    mockSaveImage = vi.fn().mockImplementation(
+      () =>
+        new Promise<{ filename: string }>((resolve) => {
+          resolveUpload = resolve;
+        }),
+    );
+
+    render(<ImageUploadField {...baseProps} onUpload={vi.fn()} />);
+    const button = screen.getByRole('button', { name: 'Upload logo' });
+
+    dropFile(button, makeFile('logo.png', 'image/png'));
+    // The control disables itself once the upload starts.
+    await waitFor(() => expect(button).toBeDisabled());
+
+    dropFile(button, makeFile('logo-2.png', 'image/png'));
+
+    expect(mockSaveImage).toHaveBeenCalledTimes(1);
+    resolveUpload({ filename: 'logo.png' });
+  });
+
+  // Edge: a valid file whose browser-reported MIME is blank/non-`image/*` is
+  // still accepted via its extension (e.g. a `.ico` reporting empty type),
+  // keeping the drop path in sync with the picker's `accept` list.
+  it('accepts a dropped file with a non-image MIME but an accepted extension', async () => {
+    const onUpload = vi.fn();
+    render(<ImageUploadField {...baseProps} onUpload={onUpload} />);
+
+    dropFile(
+      screen.getByRole('button', { name: 'Upload logo' }),
+      makeFile('favicon.ico', ''),
+    );
+
+    await waitFor(() => expect(onUpload).toHaveBeenCalledTimes(1));
+    expect(mockSaveImage).toHaveBeenCalledTimes(1);
+  });
+
+  // Error: a non-image file is rejected before any upload is attempted.
+  it('ignores a dropped non-image file', () => {
+    const onUpload = vi.fn();
+    render(<ImageUploadField {...baseProps} onUpload={onUpload} />);
+
+    dropFile(
+      screen.getByRole('button', { name: 'Upload logo' }),
+      makeFile('notes.txt', 'text/plain'),
+    );
+
+    expect(mockSaveImage).not.toHaveBeenCalled();
+    expect(onUpload).not.toHaveBeenCalled();
+  });
+
+  describe('accessibility', () => {
+    it('passes axe audit', async () => {
+      const { container } = render(
+        <ImageUploadField {...baseProps} onUpload={vi.fn()} label="Logo" />,
+      );
+      await checkAccessibility(container);
+    });
+  });
+});

services/platform/app/features/settings/branding/components/image-upload-field.tsx

@@ -3,7 +3,7 @@
 import { VStack } from '@tale/ui/layout';
 import { Spinner } from '@tale/ui/spinner';
 import { Text } from '@tale/ui/text';
-import { Plus, X } from 'lucide-react';
+import { Plus, Upload, X } from 'lucide-react';
 import { useCallback, useEffect, useRef, useState } from 'react';
 
 import { Image } from '@/app/components/ui/data-display/image';
@@ -12,6 +12,17 @@ import { cn } from '@/lib/utils/cn';
 import { useSaveImage } from '../hooks/mutations';
 
 const ACCEPTED_IMAGE_TYPES = '.png,.svg,.jpg,.jpeg,.webp,.ico';
+const ACCEPTED_EXTENSIONS = ['.png', '.svg', '.jpg', '.jpeg', '.webp', '.ico'];
+
+// Mirror the `<input accept>` list for the drag-and-drop path. Some valid
+// uploads report an empty or non-`image/*` MIME type (e.g. a `.ico` whose
+// browser-reported type is blank), so fall back to the file extension rather
+// than rejecting them — keeping picker and drop acceptance in sync.
+function isAcceptedImage(file: File): boolean {
+  if (file.type.startsWith('image/')) return true;
+  const name = file.name.toLowerCase();
+  return ACCEPTED_EXTENSIONS.some((ext) => name.endsWith(ext));
+}
 
 interface ImageUploadFieldProps {
   organizationId: string;
@@ -37,6 +48,7 @@ export function ImageUploadField({
   ariaLabel,
 }: ImageUploadFieldProps) {
   const [isUploading, setIsUploading] = useState(false);
+  const [isDragging, setIsDragging] = useState(false);
   const [previewUrl, setPreviewUrl] = useState<string | null>(null);
   const [isRemoved, setIsRemoved] = useState(false);
   const fileInputRef = useRef<HTMLInputElement>(null);
@@ -65,11 +77,8 @@ export function ImageUploadField({
     fileInputRef.current?.click();
   }, []);
 
-  const handleFileChange = useCallback(
-    async (e: React.ChangeEvent<HTMLInputElement>) => {
-      const file = e.target.files?.[0];
-      if (!file) return;
-
+  const uploadFile = useCallback(
+    async (file: File) => {
       if (objectUrlRef.current) {
         URL.revokeObjectURL(objectUrlRef.current);
       }
@@ -79,6 +88,7 @@ export function ImageUploadField({
       setPreviewUrl(objectUrl);
       onPreviewUrlChange?.(objectUrl);
       setIsRemoved(false);
+      setIsDragging(false);
       setIsUploading(true);
 
       try {
@@ -113,6 +123,38 @@ export function ImageUploadField({
     [organizationId, saveImage, imageType, onUpload, onPreviewUrlChange],
   );
 
+  const handleFileChange = useCallback(
+    (e: React.ChangeEvent<HTMLInputElement>) => {
+      const file = e.target.files?.[0];
+      if (file) void uploadFile(file);
+    },
+    [uploadFile],
+  );
+
+  const handleDragOver = useCallback(
+    (e: React.DragEvent) => {
+      e.preventDefault();
+      if (!isUploading) setIsDragging(true);
+    },
+    [isUploading],
+  );
+
+  const handleDragLeave = useCallback((e: React.DragEvent) => {
+    e.preventDefault();
+    setIsDragging(false);
+  }, []);
+
+  const handleDrop = useCallback(
+    (e: React.DragEvent) => {
+      e.preventDefault();
+      setIsDragging(false);
+      if (isUploading) return;
+      const file = e.dataTransfer.files?.[0];
+      if (file && isAcceptedImage(file)) void uploadFile(file);
+    },
+    [isUploading, uploadFile],
+  );
+
   const handleRemove = useCallback(() => {
     setPreviewUrl(null);
     onPreviewUrlChange?.(null);
@@ -132,43 +174,55 @@ export function ImageUploadField({
         <button
           type="button"
           onClick={handleClick}
+          onDragOver={handleDragOver}
+          onDragLeave={handleDragLeave}
+          onDrop={handleDrop}
           disabled={isUploading}
           className={cn(
-            'border-border flex items-center justify-center overflow-clip rounded-lg border bg-background shadow-xs',
+            'group border-border ring-offset-background relative flex cursor-pointer items-center justify-center overflow-clip rounded-lg border bg-background shadow-xs transition-all duration-150',
+            'hover:border-border-strong hover:bg-bg-elevated',
+            'focus-visible:ring-ring focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:outline-none',
+            'active:scale-[0.97] active:duration-75 motion-reduce:transition-none motion-reduce:active:scale-100',
             sizeClasses,
+            isDragging && 'border-accent-base ring-accent-base ring-1',
             isUploading && 'cursor-wait opacity-60',
           )}
           aria-label={ariaLabel}
         >
           {isUploading ? (
             <Spinner className="size-4" />
           ) : displayUrl ? (
-            previewUrl ? (
-              <img
-                src={previewUrl}
-                alt=""
-                className="size-full object-contain"
-                width={48}
-                height={48}
-              />
-            ) : (
-              <Image
-                src={displayUrl}
-                alt=""
-                className="size-full object-contain"
-                width={48}
-                height={48}
-              />
-            )
+            <>
+              {previewUrl ? (
+                <img
+                  src={previewUrl}
+                  alt=""
+                  className="pointer-events-none size-full object-contain"
+                  width={48}
+                  height={48}
+                />
+              ) : (
+                <Image
+                  src={displayUrl}
+                  alt=""
+                  className="pointer-events-none size-full object-contain"
+                  width={48}
+                  height={48}
+                />
+              )}
+              <span className="bg-foreground/60 pointer-events-none absolute inset-0 flex items-center justify-center opacity-0 transition-opacity duration-150 group-hover:opacity-100 group-focus-visible:opacity-100 motion-reduce:transition-none">
+                <Upload className="text-background size-4 shrink-0" />
+              </span>
+            </>
           ) : (
-            <Plus className="text-muted-foreground size-4 shrink-0" />
+            <Plus className="text-muted-foreground group-hover:text-foreground pointer-events-none size-4 shrink-0 transition-colors duration-150 motion-reduce:transition-none" />
           )}
         </button>
         {displayUrl && !isUploading && onRemove && (
           <button
             type="button"
             onClick={handleRemove}
-            className="bg-foreground text-background absolute -top-1 -right-1 flex size-4 items-center justify-center rounded-full"
+            className="bg-foreground text-background ring-offset-background focus-visible:ring-ring absolute -top-1 -right-1 flex size-4 cursor-pointer items-center justify-center rounded-full transition-transform duration-150 hover:scale-110 focus-visible:ring-1 focus-visible:ring-offset-2 focus-visible:outline-none motion-reduce:transition-none motion-reduce:hover:scale-100"
             aria-label={`Remove ${label ?? 'image'}`}
           >
             <X className="size-2.5" />
@@ -187,6 +241,7 @@ export function ImageUploadField({
         onChange={handleFileChange}
         className="hidden"
         tabIndex={-1}
+        aria-label={ariaLabel}
       />
     </VStack>
   );