fix(ui): MCP / skill detail as a dialog; delete-from-agent confirm label

apps/desktop/src/renderer/components/ui/DetailDialog.tsx

@@ -0,0 +1,124 @@
+// A modal for a selected item's detail (an MCP server, a skill): a fixed header with the
+// title and a close button, and a body that scrolls on its own. Opening it from a long list
+// keeps the detail in view instead of appending it below the list.
+//
+// Escape and a backdrop click close it. Callers that stack another dialog on top should
+// unmount this one meanwhile (its focus trap would otherwise hold Tab).
+
+import { useEffect, type CSSProperties, type ReactNode } from 'react';
+import { elevation, fontSize, radius, space, token } from '../../themes/tokens';
+import { hoverBackground } from '../../lib/hover';
+import { noDragRegion, useModalFocus } from './modal';
+
+export function DetailDialog({
+  labelId,
+  title,
+  closeLabel,
+  onClose,
+  children,
+}: {
+  /** id of the heading inside `title`, for aria-labelledby. */
+  labelId: string;
+  title: ReactNode;
+  closeLabel: string;
+  onClose: () => void;
+  children: ReactNode;
+}) {
+  const panelRef = useModalFocus<HTMLDivElement>();
+
+  useEffect(() => {
+    const onKeyDown = (e: KeyboardEvent): void => {
+      if (e.key === 'Escape' && !e.defaultPrevented) onClose();
+    };
+    window.addEventListener('keydown', onKeyDown);
+    return () => window.removeEventListener('keydown', onKeyDown);
+  }, [onClose]);
+
+  return (
+    <div style={overlayStyle} onClick={onClose}>
+      <div
+        ref={panelRef}
+        role="dialog"
+        aria-modal="true"
+        aria-labelledby={labelId}
+        style={panelStyle}
+        onClick={(e) => e.stopPropagation()}
+      >
+        <div style={headerStyle}>
+          <div style={{ flex: 1, minWidth: 0 }}>{title}</div>
+          <button
+            type="button"
+            onClick={onClose}
+            aria-label={closeLabel}
+            title={closeLabel}
+            {...hoverBackground('transparent', token('surfaceHover'))}
+            style={closeButtonStyle}
+          >
+            ×
+          </button>
+        </div>
+        <div style={bodyStyle}>{children}</div>
+      </div>
+    </div>
+  );
+}
+
+const overlayStyle: CSSProperties = {
+  position: 'fixed',
+  ...noDragRegion,
+  inset: 0,
+  background: token('overlay'),
+  display: 'grid',
+  placeItems: 'center',
+  padding: space['2xl'],
+  zIndex: 20,
+};
+
+const panelStyle: CSSProperties = {
+  width: 'min(760px, 100%)',
+  maxHeight: '100%',
+  boxSizing: 'border-box',
+  overflow: 'hidden',
+  display: 'flex',
+  flexDirection: 'column',
+  background: token('bg'),
+  color: token('text'),
+  border: `1px solid ${token('border')}`,
+  borderRadius: radius.xl,
+  boxShadow: elevation('modal'),
+  outline: 'none',
+};
+
+const headerStyle: CSSProperties = {
+  display: 'flex',
+  alignItems: 'flex-start',
+  gap: space.md,
+  flexShrink: 0,
+  padding: `${space.xl}px ${space.xl}px ${space.md}px`,
+  borderBottom: `1px solid ${token('border')}`,
+};
+
+const bodyStyle: CSSProperties = {
+  flex: 1,
+  minHeight: 0,
+  overflowY: 'auto',
+  overscrollBehavior: 'contain',
+  padding: `${space.lg}px ${space.xl}px ${space.xl}px`,
+};
+
+const closeButtonStyle: CSSProperties = {
+  display: 'inline-flex',
+  alignItems: 'center',
+  justifyContent: 'center',
+  flexShrink: 0,
+  width: 28,
+  height: 28,
+  padding: 0,
+  borderRadius: radius.sm,
+  border: 'none',
+  background: 'transparent',
+  color: token('textMuted'),
+  cursor: 'pointer',
+  fontSize: fontSize.xl,
+  lineHeight: 1,
+};

apps/desktop/src/renderer/features/mcp/McpHome.tsx

@@ -13,6 +13,7 @@
 
 import { useCallback, useEffect, useState, type CSSProperties, type FormEvent } from 'react';
 import { disabledOpacity, elevation, fontSize, radius, space, token } from '../../themes/tokens';
+import { DetailDialog } from '../../components/ui/DetailDialog';
 import { hoverBackground } from '../../lib/hover';
 import { useT, type TranslateFn } from '../../i18n';
 import { ScrollTabScreen } from '../../components/TabScreenShell';
@@ -209,9 +210,11 @@ export function McpHome() {
             }
           />
 
-          {selectedRow && (
+          {/* Hidden while the edit form is open: one modal at a time. */}
+          {selectedRow && !editing && (
             <DetailPanel
               row={selectedRow}
+              onClose={() => (confirm ? setConfirm(null) : setSelected(null))}
               agents={columns}
               busy={busy}
               confirm={confirm}
@@ -460,6 +463,7 @@ function CellChip({ info, cell, t }: { info: McpAgentInfo; cell: McpMatrixRow['a
 
 function DetailPanel({
   row,
+  onClose,
   agents,
   busy,
   confirm,
@@ -478,6 +482,7 @@ function DetailPanel({
   onGateway,
 }: {
   row: McpMatrixRow;
+  onClose: () => void;
   agents: McpAgentInfo[];
   busy: boolean;
   confirm: Confirm | null;
@@ -503,11 +508,17 @@ function DetailPanel({
   const copyCount = (lib ? 1 : 0) + agents.filter((a) => row.agents[a.agentId]?.entry).length;
 
   return (
-    <section style={detailStyle}>
-      <div style={{ display: 'flex', alignItems: 'baseline', gap: space.md, flexWrap: 'wrap' }}>
-        <h2 style={{ margin: 0, fontSize: fontSize.xl }}>{row.name}</h2>
-        {lib && <span style={badgeStyle}>{t(`mcp.transport.${lib.transport}`)}</span>}
-      </div>
+    <DetailDialog
+      labelId="mcp-detail-title"
+      closeLabel={t('mcp.diff.close')}
+      onClose={onClose}
+      title={
+        <div style={{ display: 'flex', alignItems: 'baseline', gap: space.md, flexWrap: 'wrap' }}>
+          <h2 id="mcp-detail-title" style={{ margin: 0, fontSize: fontSize.xl, overflowWrap: 'anywhere' }}>{row.name}</h2>
+          {lib && <span style={badgeStyle}>{t(`mcp.transport.${lib.transport}`)}</span>}
+        </div>
+      }
+    >
       {lib ? (
         <>
           <div style={summaryStyle} title={mcpSummary(lib)}>{mcpSummary(lib)}</div>
@@ -644,7 +655,7 @@ function DetailPanel({
                   onClick={() =>
                     setConfirm({
                       text: copyCount <= 1 ? t('mcp.confirm.deleteAgentLast') : t('mcp.confirm.deleteAgent'),
-                      confirmLabel: t('mcp.actions.delete'),
+                      confirmLabel: t('mcp.actions.deleteAgent'),
                       run: () => onDeleteAgent(a.agentId),
                     })
                   }
@@ -667,7 +678,7 @@ function DetailPanel({
           }
         />
       )}
-    </section>
+    </DetailDialog>
   );
 }
 
@@ -1279,6 +1290,12 @@ const confirmBarStyle: CSSProperties = {
   padding: '10px 14px',
   marginBottom: space.lg,
   borderRadius: radius.md,
+  // Pinned to the top of the dialog body so it stays in view when it was opened from a
+  // row further down.
+  position: 'sticky',
+  top: 0,
+  zIndex: 1,
+  background: token('bg'),
   border: `1px solid ${token('danger')}`,
 };
 

apps/desktop/src/renderer/features/skills/SkillsHome.tsx

@@ -12,6 +12,7 @@
 
 import { useEffect, useMemo, useState } from 'react';
 import { disabledOpacity, fontSize, radius, space, token } from '../../themes/tokens';
+import { DetailDialog } from '../../components/ui/DetailDialog';
 import { hoverBackground } from '../../lib/hover';
 import { useI18n, type Locale, type TranslateFn } from '../../i18n';
 import { useSkills } from '../../hooks/useSkills';
@@ -252,6 +253,7 @@ export function SkillsHome() {
         {selectedRow && (
           <DetailPanel
             row={selectedRow}
+            onClose={() => (confirm ? setConfirm(null) : setSelected(null))}
             agents={availableAgents}
             agentName={agentName}
             busy={busy}
@@ -399,6 +401,7 @@ function Matrix({
 
 function DetailPanel({
   row,
+  onClose,
   agents,
   agentName,
   busy,
@@ -418,6 +421,7 @@ function DetailPanel({
   locale,
 }: {
   row: SkillMatrixRow;
+  onClose: () => void;
   agents: { id: AgentId; name: string }[];
   agentName: (id: AgentId) => string;
   busy: boolean;
@@ -446,16 +450,22 @@ function DetailPanel({
     (hasCentral ? 1 : 0) + agents.filter((a) => row.agents[a.id]?.entry != null).length;
 
   return (
-    <section style={detailStyle}>
-      <div style={{ display: 'flex', alignItems: 'baseline', gap: space.md, flexWrap: 'wrap' }}>
-        <h2 style={{ margin: 0, fontSize: fontSize.xl }}>{row.name}</h2>
-        {hasCentral && row.central && (
-          <span style={{ fontSize: fontSize.sm, color: token('textMuted') }}>
-            {numberFormat.format(row.central.fileCount)} {t('skills.meta.files')} ·{' '}
-            {formatBytes(row.central.totalBytes)}
-          </span>
-        )}
-      </div>
+    <DetailDialog
+      labelId="skill-detail-title"
+      closeLabel={t('skills.close')}
+      onClose={onClose}
+      title={
+        <div style={{ display: 'flex', alignItems: 'baseline', gap: space.md, flexWrap: 'wrap' }}>
+          <h2 id="skill-detail-title" style={{ margin: 0, fontSize: fontSize.xl, overflowWrap: 'anywhere' }}>{row.name}</h2>
+          {hasCentral && row.central && (
+            <span style={{ fontSize: fontSize.sm, color: token('textMuted') }}>
+              {numberFormat.format(row.central.fileCount)} {t('skills.meta.files')} ·{' '}
+              {formatBytes(row.central.totalBytes)}
+            </span>
+          )}
+        </div>
+      }
+    >
       <p style={{ margin: '4px 0 16px', color: token('textMuted'), fontSize: fontSize.base }}>
         {description || t('skills.detail.noDescription')}
       </p>
@@ -616,7 +626,7 @@ function DetailPanel({
                         copyCount <= 1
                           ? t('skills.confirm.deleteAgentLast')
                           : t('skills.confirm.deleteAgent'),
-                      confirmLabel: t('skills.actions.delete'),
+                      confirmLabel: t('skills.actions.deleteAgent'),
                       run: async () => onDeleteAgent(a.id),
                     })
                   }
@@ -644,7 +654,7 @@ function DetailPanel({
           t={t}
         />
       )}
-    </section>
+    </DetailDialog>
   );
 }
 
@@ -1117,21 +1127,19 @@ const bodyCellStyle = {
   whiteSpace: 'nowrap',
 } as const;
 
-const detailStyle = {
-  marginTop: 24,
-  padding: space['2xl'],
-  border: `1px solid ${token('border')}`,
-  borderRadius: radius.lg,
-  background: token('surface'),
-} as const;
-
 const confirmBarStyle = {
   display: 'flex',
   alignItems: 'center',
   gap: space.md,
   padding: '10px 14px',
   marginBottom: 16,
   borderRadius: radius.md,
+  // Pinned to the top of the dialog body so it stays in view when it was opened from a
+  // row further down.
+  position: 'sticky',
+  top: 0,
+  zIndex: 1,
+  background: token('bg'),
   border: `1px solid ${token('danger')}`,
 } as const;