Browse Source

Restore the pointer cursor on interactive controls (#2791)

    Hovering most of Bambuddy gave an arrow rather than a hand. Not everywhere,
    which is what made it read as sloppiness rather than a bug: the update pill was
    inert while the buttons beside it were fine, a bed or nozzle tile responded but
    the history-graph button in its corner did not, and dropdowns went either way
    with no pattern behind it.

    The pattern was there. Tailwind v3's Preflight set `button { cursor: pointer }`.
    v4 dropped it to match the browser default, which for a button is `default`.
    Bambuddy has been on v4 since the frontend was built, and `src/index.css` never
    had a base layer restoring it, so a button only looked clickable where someone
    had written `cursor-pointer` by hand. 15 of 934 had. 0 of 149 selects, and 19
    of 130 checkbox/radio inputs. The 233 ad-hoc `cursor-pointer` usages are why it
    looked arbitrary instead of uniformly broken.

    One `@layer base` rule now covers button, select, checkbox, radio, summary and
    [role=button]. base sits below utilities, so `cursor-not-allowed` and the
    `disabled:cursor-*` variants still win; the `:not(:disabled)` guard catches the
    disabled controls that carry no such utility. Verified against the built bundle
    rather than the source -- the rule lands inside @layer base, and
    `.cursor-not-allowed` is emitted after it.

    Click-outside backdrops are deliberately excluded. 90 of the 96 remaining
    onClick divs are `fixed inset-0` overlays; a full-screen sheet advertising
    itself as a button is worse than one that says nothing. Of the rest, 50 are
    stopPropagation wrappers and 3 are the temperature tiles, which already set the
    cursor through `statusControlClass` -- which is exactly why those tiles worked
    while the button nested inside them did not. That left two real ones: Card, now
    conditional on an onClick actually being passed, and the queue card, whose
    existing `sm:cursor-default` kept the desktop intent.

    Separately, from the same report. FilamentHoverCard draws the slot menu twice,
    and the two paths had drifted into opposite orders: Configure above Assign Spool
    on an empty slot, the reverse on a filled one, so the menu reshuffled itself
    depending on whether the slot held filament. Both now lead with the spool
    action. Tests assert the order on each path, so one can no longer move without
    the other -- checked by reinstating the old order and confirming the empty-slot
    test fails.

    Those buttons also used justify-center, which centred each label independently
    and left the icons in a ragged column; they are justify-start now. Their hover
    was a 10% opacity step that was very hard to see, now 20%. And the favourites
    star previews yellow on hover, suppressed when the user lacks archives:update.
maziggy 3 weeks ago
parent
commit
50ec1872cb

+ 47 - 0
frontend/src/__tests__/components/FilamentHoverCard.test.tsx

@@ -152,6 +152,28 @@ describe('FilamentHoverCard', () => {
   // removed that gate so users who don't want to scan via SpoolBuddy NFC
   // can still pick a BL spool from inventory the same way they pick a
   // third-party one.
+  // Paired with the EmptySlotHoverCard assertion below (#2791) — together
+  // they pin the two render paths to the same Assign-then-Configure order.
+  it('lists Assign Spool above Configure (#2791)', async () => {
+    renderWithHover(
+      <FilamentHoverCard
+        data={baseFilamentData}
+        inventory={{ assignedSpool: null, onAssignSpool: vi.fn() }}
+        configureSlot={{ enabled: true, onConfigure: vi.fn() }}
+      >
+        <div>trigger</div>
+      </FilamentHoverCard>
+    );
+    vi.advanceTimersByTime(100);
+    await waitFor(() => expect(screen.getByText(/assign spool/i)).toBeInTheDocument());
+
+    const assign = screen.getByText(/assign spool/i);
+    const configure = screen.getByText(/^configure$/i);
+    expect(assign.compareDocumentPosition(configure)).toBe(
+      Node.DOCUMENT_POSITION_FOLLOWING
+    );
+  });
+
   describe('inventory section vendor visibility (#1133)', () => {
     it('shows the assign-spool button on a Bambu Lab slot when the spool is unassigned', async () => {
       const onAssign = vi.fn();
@@ -485,6 +507,31 @@ describe('EmptySlotHoverCard (#1133)', () => {
     expect(onAssign).toHaveBeenCalledTimes(1);
   });
 
+  // #2791: the empty-slot and filled-slot cards are separate render paths
+  // that had drifted into opposite orders, so the menu reshuffled itself
+  // depending on whether the slot happened to hold filament. Both now put
+  // the spool action above the slot action; assert it on both paths so the
+  // two can't drift apart again.
+  it('lists Assign Spool above Configure, matching the filled-slot card (#2791)', async () => {
+    const result = render(
+      <EmptySlotHoverCard
+        configureSlot={{ enabled: true, onConfigure: vi.fn() }}
+        onAssignSpool={vi.fn()}
+      >
+        <div>trigger</div>
+      </EmptySlotHoverCard>
+    );
+    fireEvent.mouseEnter(result.container.firstElementChild as HTMLElement);
+    vi.advanceTimersByTime(100);
+    await waitFor(() => expect(screen.getByText(/assign spool/i)).toBeInTheDocument());
+
+    const assign = screen.getByText(/assign spool/i);
+    const configure = screen.getByText(/^configure$/i);
+    expect(assign.compareDocumentPosition(configure)).toBe(
+      Node.DOCUMENT_POSITION_FOLLOWING
+    );
+  });
+
   // Same z-[60]-over-a-z-50-dialog problem as FilamentHoverCard (#2631).
   describe('dismissal when an action opens a dialog (#2631)', () => {
     it('closes the card when Configure is pressed, and still configures', async () => {

+ 1 - 1
frontend/src/components/Card.tsx

@@ -25,7 +25,7 @@ interface CardSectionProps {
 export function Card({ children, className = '', onClick, onContextMenu, ...rest }: CardProps) {
   return (
     <div
-      className={`bg-bambu-dark-secondary rounded-xl border border-bambu-dark-tertiary card-shadow ${className}`}
+      className={`bg-bambu-dark-secondary rounded-xl border border-bambu-dark-tertiary card-shadow ${onClick ? 'cursor-pointer' : ''} ${className}`}
       onClick={onClick}
       onContextMenu={onContextMenu}
       {...rest}

+ 1 - 1
frontend/src/components/ContextMenu.tsx

@@ -265,7 +265,7 @@ export function ContextMenu({ x, y, items, onClose }: ContextMenuProps) {
               }}
               disabled={item.disabled}
               title={item.title}
-              className={`w-full flex items-center gap-2 px-3 py-2 text-sm text-left transition-colors ${
+              className={`group w-full flex items-center gap-2 px-3 py-2 text-sm text-left transition-colors ${
                 item.disabled
                   ? 'text-bambu-gray cursor-not-allowed'
                   : item.danger

+ 22 - 17
frontend/src/components/FilamentHoverCard.tsx

@@ -344,7 +344,7 @@ export function FilamentHoverCard({ data, children, disabled, className = '', sp
                           dismiss();
                           navigate(`/inventory?spool=${spoolman.linkedSpoolId}`);
                         }}
-                        className="w-full flex items-center justify-center gap-1.5 px-2 py-1.5 text-xs font-medium rounded transition-colors bg-bambu-green/20 hover:bg-bambu-green/30 text-bambu-green"
+                        className="w-full flex items-center justify-start gap-1.5 px-2 py-1.5 text-xs font-medium rounded transition-colors bg-bambu-green/20 hover:bg-bambu-green/40 text-bambu-green"
                         title={t('inventory.openInInventory')}
                       >
                         <Package className="w-3.5 h-3.5" />
@@ -397,7 +397,7 @@ export function FilamentHoverCard({ data, children, disabled, className = '', sp
                             dismiss();
                             navigate(`/inventory?spool=${inventory.assignedSpool!.id}`);
                           }}
-                          className="w-full flex items-center justify-center gap-1.5 px-2 py-1.5 text-xs font-medium rounded transition-colors bg-bambu-green/20 hover:bg-bambu-green/30 text-bambu-green"
+                          className="w-full flex items-center justify-start gap-1.5 px-2 py-1.5 text-xs font-medium rounded transition-colors bg-bambu-green/20 hover:bg-bambu-green/40 text-bambu-green"
                           title={t('inventory.openInInventory')}
                         >
                           <Package className="w-3.5 h-3.5" />
@@ -411,7 +411,7 @@ export function FilamentHoverCard({ data, children, disabled, className = '', sp
                             dismiss();
                             inventory.onUnassignSpool?.();
                           }}
-                          className="w-full flex items-center justify-center gap-1.5 px-2 py-1.5 text-xs font-medium rounded transition-colors bg-red-100 dark:bg-red-500/20 hover:bg-red-200 dark:hover:bg-red-500/30 text-red-700 dark:text-red-400"
+                          className="w-full flex items-center justify-start gap-1.5 px-2 py-1.5 text-xs font-medium rounded transition-colors bg-red-100 dark:bg-red-500/20 hover:bg-red-200 dark:hover:bg-red-500/40 text-red-700 dark:text-red-400"
                         >
                           <Unlink className="w-3.5 h-3.5" />
                           {t('inventory.unassignSpool')}
@@ -426,8 +426,8 @@ export function FilamentHoverCard({ data, children, disabled, className = '', sp
                         inventory.onAssignSpool?.();
                       }}
                       disabled={!!inventory.isAssigned}
-                      className={`w-full flex items-center justify-center gap-1.5 px-2 py-1.5 text-xs font-medium rounded transition-colors bg-bambu-blue/20 text-bambu-blue ${
-                        inventory.isAssigned ? 'opacity-50 cursor-not-allowed' : 'hover:bg-bambu-blue/30'
+                      className={`w-full flex items-center justify-start gap-1.5 px-2 py-1.5 text-xs font-medium rounded transition-colors bg-bambu-blue/20 text-bambu-blue ${
+                        inventory.isAssigned ? 'opacity-50 cursor-not-allowed' : 'hover:bg-bambu-blue/40'
                       }`}
                     >
                       <Package className="w-3.5 h-3.5" />
@@ -446,7 +446,7 @@ export function FilamentHoverCard({ data, children, disabled, className = '', sp
                       dismiss();
                       configureSlot.onConfigure?.();
                     }}
-                    className="w-full flex items-center justify-center gap-1.5 px-2 py-1.5 text-xs font-medium rounded transition-colors bg-bambu-blue/20 hover:bg-bambu-blue/30 text-bambu-blue"
+                    className="w-full flex items-center justify-start gap-1.5 px-2 py-1.5 text-xs font-medium rounded transition-colors bg-bambu-blue/20 hover:bg-bambu-blue/40 text-bambu-blue"
                     title={t('ams.configureSlot')}
                   >
                     <Settings2 className="w-3.5 h-3.5" />
@@ -506,7 +506,7 @@ export function FilamentHoverCard({ data, children, disabled, className = '', sp
                     spoolman?.onUnlinkSpool?.();
                     setShowUnlinkConfirm(false);
                   }}
-                  className="flex-1 px-3 py-2 text-sm font-medium rounded transition-colors bg-red-100 dark:bg-red-500/20 hover:bg-red-200 dark:hover:bg-red-500/30 text-red-700 dark:text-red-400"
+                  className="flex-1 px-3 py-2 text-sm font-medium rounded transition-colors bg-red-100 dark:bg-red-500/20 hover:bg-red-200 dark:hover:bg-red-500/40 text-red-700 dark:text-red-400"
                 >
                   {t('inventory.unassignSpool')}
                 </button>
@@ -622,6 +622,20 @@ export function EmptySlotHoverCard({ children, className = '', configureSlot, on
             {/* Configure slot button */}
             {(configureSlot?.enabled || onAssignSpool || actions) && (
               <div className="px-2 pb-2 space-y-1">
+                {/* Assign before Configure, matching the filled-slot card
+                    above (#2791).  The two cards are separate render paths
+                    and had drifted into opposite orders, so the menu
+                    reshuffled itself depending on whether the slot happened
+                    to hold filament. */}
+                {onAssignSpool && (
+                  <button
+                    onClick={(e) => { e.stopPropagation(); dismiss(); onAssignSpool(); }}
+                    className="w-full flex items-center justify-start gap-1.5 px-2 py-1.5 text-xs font-medium rounded transition-colors bg-bambu-blue/20 hover:bg-bambu-blue/40 text-bambu-blue"
+                  >
+                    <Package className="w-3.5 h-3.5" />
+                    {t('inventory.assignSpool')}
+                  </button>
+                )}
                 {configureSlot?.enabled && (
                   <button
                     onClick={(e) => {
@@ -629,22 +643,13 @@ export function EmptySlotHoverCard({ children, className = '', configureSlot, on
                       dismiss();
                       configureSlot.onConfigure?.();
                     }}
-                    className="w-full flex items-center justify-center gap-1.5 px-2 py-1.5 text-xs font-medium rounded transition-colors bg-bambu-blue/20 hover:bg-bambu-blue/30 text-bambu-blue"
+                    className="w-full flex items-center justify-start gap-1.5 px-2 py-1.5 text-xs font-medium rounded transition-colors bg-bambu-blue/20 hover:bg-bambu-blue/40 text-bambu-blue"
                     title={t('ams.configureSlot')}
                   >
                     <Settings2 className="w-3.5 h-3.5" />
                     {t('ams.configure')}
                   </button>
                 )}
-                {onAssignSpool && (
-                  <button
-                    onClick={(e) => { e.stopPropagation(); dismiss(); onAssignSpool(); }}
-                    className="w-full flex items-center justify-center gap-1.5 px-2 py-1.5 text-xs font-medium rounded transition-colors bg-bambu-blue/20 hover:bg-bambu-blue/30 text-bambu-blue"
-                  >
-                    <Package className="w-3.5 h-3.5" />
-                    {t('inventory.assignSpool')}
-                  </button>
-                )}
                 {actions && (
                   <div className="pt-1 mt-1 border-t border-bambu-dark-tertiary space-y-1">
                     {actions}

+ 22 - 0
frontend/src/index.css

@@ -26,6 +26,28 @@
 /* Enable class-based dark mode for Tailwind v4 */
 @custom-variant dark (&:where(.dark, .dark *));
 
+/* Restore the pointer cursor on interactive controls (#2791).  Tailwind v3's
+   Preflight set `button { cursor: pointer }`; v4 dropped it to match the
+   browser default of `cursor: default`, so every button in the app looked
+   unclickable unless someone remembered to add `cursor-pointer` by hand.
+   Only a handful of the ~930 buttons did, which is why the UI felt
+   inconsistent rather than uniformly wrong.
+
+   This lives in `base`, the lowest of Tailwind's cascade layers, so the
+   `cursor-not-allowed` / `disabled:cursor-*` utilities dotted around the
+   codebase still win.  The `:not(:disabled)` guard covers the elements that
+   are disabled without also carrying such a utility. */
+@layer base {
+  button:not(:disabled),
+  select:not(:disabled),
+  summary,
+  input[type="checkbox"]:not(:disabled),
+  input[type="radio"]:not(:disabled),
+  [role="button"]:not([aria-disabled="true"]) {
+    cursor: pointer;
+  }
+}
+
 @theme {
   /* Accent colors - use CSS variables for theming */
   --color-bambu-green: var(--accent);

+ 4 - 2
frontend/src/pages/ArchivesPage.tsx

@@ -740,7 +740,8 @@ function ArchiveCard({
     { label: '', divider: true, onClick: () => {} },
     {
       label: archive.is_favorite ? t('archives.menu.removeFromFavorites') : t('archives.menu.addToFavorites'),
-      icon: <Star className={`w-4 h-4 ${archive.is_favorite ? 'fill-yellow-400 text-yellow-400' : ''}`} />,
+      // Preview the favourited state on hover so the row reads as clickable (#2791).
+      icon: <Star className={`w-4 h-4 ${archive.is_favorite ? 'fill-yellow-400 text-yellow-400' : canModify('archives', 'update', archive.created_by_id) ? 'group-hover:text-yellow-400' : ''}`} />,
       onClick: () => favoriteMutation.mutate(),
       disabled: !canModify('archives', 'update', archive.created_by_id),
       title: !canModify('archives', 'update', archive.created_by_id) ? t('archives.permission.noUpdateArchives') : undefined,
@@ -2138,7 +2139,8 @@ function ArchiveListRow({
     { label: '', divider: true, onClick: () => {} },
     {
       label: archive.is_favorite ? t('archives.menu.removeFromFavorites') : t('archives.menu.addToFavorites'),
-      icon: <Star className={`w-4 h-4 ${archive.is_favorite ? 'fill-yellow-400 text-yellow-400' : ''}`} />,
+      // Preview the favourited state on hover so the row reads as clickable (#2791).
+      icon: <Star className={`w-4 h-4 ${archive.is_favorite ? 'fill-yellow-400 text-yellow-400' : canModify('archives', 'update', archive.created_by_id) ? 'group-hover:text-yellow-400' : ''}`} />,
       onClick: () => favoriteMutation.mutate(),
       disabled: !canModify('archives', 'update', archive.created_by_id),
       title: !canModify('archives', 'update', archive.created_by_id) ? t('archives.permission.noUpdateArchives') : undefined,

+ 1 - 1
frontend/src/pages/QueuePage.tsx

@@ -466,7 +466,7 @@ function SortableQueueItem({
         ${isPrinting ? 'border-blue-500/30 bg-gradient-to-r from-blue-500/5 to-transparent' : ''}
         ${isSelected && isMobileSelectable ? 'sm:border-bambu-dark-tertiary border-bambu-green/40' : ''}
         ${!isSelected && !isPrinting ? 'border-bambu-dark-tertiary hover:border-bambu-dark-tertiary/80' : ''}
-        ${isMobileSelectable ? 'sm:cursor-default' : ''}
+        ${isMobileSelectable ? 'cursor-pointer sm:cursor-default' : ''}
       `}
       onClick={isMobileSelectable ? () => {
         if (window.innerWidth < 640) onToggleSelect();