fix(web): address UI polish review feedback

2026-08-04 16:25:09 +08:00
parent b9f4e46462
commit e9d76dae64
23 changed files with 174 additions and 116 deletions
--- a/packages/client/ui-primitives/src/Tooltip.tsx
+++ b/packages/client/ui-primitives/src/Tooltip.tsx
@@ -47,19 +47,27 @@ export function Tooltip({ label, side = 'right', delayMs = 0, disabled = false,
  const [pos, setPos] = useState<{ x: number; y: number } | null>(null)
  const bubble = useRef<HTMLSpanElement | null>(null)
  // Horizontal viewport clamp: fixed positioning knows nothing about edges, so
-  // a centered bubble near the right edge would clip. Measured after paint and
-  // written straight to the style (no state), so it cannot re-trigger itself.
-  // EDGE_MARGIN keeps the bubble 12px off the viewport edges instead of flush.
+  // a centered bubble near the right edge would clip. Each measurement resets
+  // the base position before applying a direct style offset, allowing a shorter
+  // label or wider viewport to release a previous clamp without another render.
  useLayoutEffect(() => {
-    const el = bubble.current
-    if (el === null || pos === null) return
-    const EDGE_MARGIN = 12
-    const r = el.getBoundingClientRect()
-    let dx = 0
-    if (r.right > window.innerWidth - EDGE_MARGIN) dx = window.innerWidth - EDGE_MARGIN - r.right
-    if (r.left + dx < EDGE_MARGIN) dx = EDGE_MARGIN - r.left
-    if (dx !== 0) el.style.left = `${pos.x + dx}px`
-  }, [pos])
+    if (pos === null) return
+    const clamp = () => {
+      const el = bubble.current
+      /* v8 ignore next -- pos is set only while the bubble is mounted. */
+      if (el === null) return
+      const EDGE_MARGIN = 12
+      el.style.left = `${pos.x}px`
+      const r = el.getBoundingClientRect()
+      let dx = 0
+      if (r.right > window.innerWidth - EDGE_MARGIN) dx = window.innerWidth - EDGE_MARGIN - r.right
+      if (r.left + dx < EDGE_MARGIN) dx = EDGE_MARGIN - r.left
+      el.style.left = `${pos.x + dx}px`
+    }
+    clamp()
+    window.addEventListener('resize', clamp)
+    return () => { window.removeEventListener('resize', clamp) }
+  }, [label, pos])
  const showTimer = useRef<ReturnType<typeof setTimeout> | null>(null)
  // Hover and focus are independent triggers: the bubble hides only after
  // BOTH clear (hovering away from a focused anchor must not drop it).
--- a/packages/client/ui-primitives/src/clipboard.ts
+++ b/packages/client/ui-primitives/src/clipboard.ts
@@ -1,6 +1,5 @@
-// Package-internal clipboard write, shared by every copy control in this
-// package (CodeBlock's code copy, TerminalBlock's output copy). Not part of the
-// public surface: consumers get the components, not the host detection.
+// Host clipboard write shared by Web UI copy controls. Success feedback stays
+// with each control; this seam only reports whether the host accepted a write.

 /**
 * Write text to the host clipboard, preferring the async Clipboard API and
--- a/packages/client/ui-primitives/src/icons/index.tsx
+++ b/packages/client/ui-primitives/src/icons/index.tsx
@@ -726,23 +726,16 @@ export const IconListPenOutline16 = ({ size = 16, className }: IconProps) => (
 /** ic_ds_goal_outline_16 (goal strip leading glyph: dartboard with a landed arrow) */
 export const IconGoalOutline16 = ({ size = 16, className }: IconProps) => (
  <svg width={size} height={size} className={className} viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg">
-    <g clipPath="url(#clip0_1498_30899)">
-      <path
-        d="M8 0C8.31451 0 8.62464 0.019379 8.92969 0.0546875C8.48228 0.403371 8.0952 0.825758 7.78809 1.30469C4.18586 1.41664 1.2998 4.37061 1.2998 8C1.2998 11.7003 4.29969 14.7002 8 14.7002C11.6297 14.7002 14.5829 11.8136 14.6943 8.21094C15.1734 7.90377 15.5956 7.51688 15.9443 7.06934C15.9797 7.37473 16 7.68512 16 8C16 12.4183 12.4183 16 8 16C3.58172 16 0 12.4183 0 8C0 3.58172 3.58172 0 8 0ZM7.0166 3.6084C7.00658 3.73765 7 3.86817 7 4C7 4.31845 7.03098 4.62973 7.08789 4.93164C5.76489 5.32438 4.7998 6.54958 4.7998 8C4.7998 9.76731 6.23269 11.2002 8 11.2002C9.45065 11.2002 10.6749 10.2345 11.0674 8.91113C11.3696 8.96818 11.6812 9 12 9C12.1315 9 12.2617 8.99239 12.3906 8.98242C11.9423 10.995 10.1477 12.5 8 12.5C5.51472 12.5 3.5 10.4853 3.5 8C3.5 5.85255 5.00435 4.05702 7.0166 3.6084Z"
-        fill="currentColor"
-      />
-      <path d="M7.5 8.62109L9.12109 7" stroke="currentColor" strokeWidth="1.3" />
-      <path
-        d="M9.08245 3.35798L11.8651 0.575334C11.895 0.545384 11.9463 0.56391 11.9502 0.606086L12.2362 3.69859C12.2384 3.72259 12.2574 3.74159 12.2814 3.74378L15.3697 4.02583C15.4119 4.02968 15.4305 4.08101 15.4005 4.11098L12.618 6.89351C12.6086 6.90289 12.5959 6.90816 12.5826 6.90816L9.11781 6.90815C9.09019 6.90816 9.06781 6.88577 9.06781 6.85816L9.06781 3.39333C9.06781 3.38007 9.07308 3.36735 9.08245 3.35798Z"
-        stroke="currentColor"
-        strokeWidth="1.3"
-      />
-    </g>
-    <defs>
-      <clipPath id="clip0_1498_30899">
-        <rect width={16} height={16} fill="currentColor" />
-      </clipPath>
-    </defs>
+    <path
+      d="M8 0C8.31451 0 8.62464 0.019379 8.92969 0.0546875C8.48228 0.403371 8.0952 0.825758 7.78809 1.30469C4.18586 1.41664 1.2998 4.37061 1.2998 8C1.2998 11.7003 4.29969 14.7002 8 14.7002C11.6297 14.7002 14.5829 11.8136 14.6943 8.21094C15.1734 7.90377 15.5956 7.51688 15.9443 7.06934C15.9797 7.37473 16 7.68512 16 8C16 12.4183 12.4183 16 8 16C3.58172 16 0 12.4183 0 8C0 3.58172 3.58172 0 8 0ZM7.0166 3.6084C7.00658 3.73765 7 3.86817 7 4C7 4.31845 7.03098 4.62973 7.08789 4.93164C5.76489 5.32438 4.7998 6.54958 4.7998 8C4.7998 9.76731 6.23269 11.2002 8 11.2002C9.45065 11.2002 10.6749 10.2345 11.0674 8.91113C11.3696 8.96818 11.6812 9 12 9C12.1315 9 12.2617 8.99239 12.3906 8.98242C11.9423 10.995 10.1477 12.5 8 12.5C5.51472 12.5 3.5 10.4853 3.5 8C3.5 5.85255 5.00435 4.05702 7.0166 3.6084Z"
+      fill="currentColor"
+    />
+    <path d="M7.5 8.62109L9.12109 7" stroke="currentColor" strokeWidth="1.3" />
+    <path
+      d="M9.08245 3.35798L11.8651 0.575334C11.895 0.545384 11.9463 0.56391 11.9502 0.606086L12.2362 3.69859C12.2384 3.72259 12.2574 3.74159 12.2814 3.74378L15.3697 4.02583C15.4119 4.02968 15.4305 4.08101 15.4005 4.11098L12.618 6.89351C12.6086 6.90289 12.5959 6.90816 12.5826 6.90816L9.11781 6.90815C9.09019 6.90816 9.06781 6.88577 9.06781 6.85816L9.06781 3.39333C9.06781 3.38007 9.07308 3.36735 9.08245 3.35798Z"
+      stroke="currentColor"
+      strokeWidth="1.3"
+    />
  </svg>
 )

--- a/packages/client/ui-primitives/src/index.ts
+++ b/packages/client/ui-primitives/src/index.ts
@@ -20,6 +20,7 @@ export { FishLogo } from './FishLogo.tsx'
 export { BrandWordmark } from './BrandWordmark.tsx'
 export { Tooltip } from './Tooltip.tsx'
 export type { TooltipSide } from './Tooltip.tsx'
+export { writeClipboard } from './clipboard.ts'
 export { JsonTree } from './JsonTree.tsx'
 export type { JsonTreeProps, JsonTreeLabels } from './JsonTree.tsx'
 export { TerminalBlock, DEFAULT_TERMINAL_MAX_LINES } from './TerminalBlock.tsx'
--- a/packages/client/ui-primitives/tests/icons.spec.tsx
+++ b/packages/client/ui-primitives/tests/icons.spec.tsx
@@ -2,7 +2,9 @@
 import { cleanup, render } from '@testing-library/react'
 import { afterEach, describe, expect, it } from 'vitest'
 import * as primitives from '@deepseek-ai/dsh-client-ui-primitives'
-import { IconApiOutline14, IconArchiveOutline20, IconFolderClose16, IconSendOutline16 } from '@deepseek-ai/dsh-client-ui-primitives'
+import {
+  IconApiOutline14, IconArchiveOutline20, IconFolderClose16, IconGoalOutline16, IconSendOutline16,
+} from '@deepseek-ai/dsh-client-ui-primitives'

 afterEach(cleanup)

@@ -44,6 +46,12 @@ describe('ic_ds_ icon set', () => {
    const archive = render(<IconArchiveOutline20 />)
    expect(archive.container.querySelector('svg')!.getAttribute('width')).toBe('20')
  })
+
+  it('renders reusable goal glyphs without document-global ids', () => {
+    const { container } = render(<><IconGoalOutline16 /><IconGoalOutline16 /></>)
+    expect(container.querySelector('[id]')).toBeNull()
+    expect(container.querySelector('[clip-path]')).toBeNull()
+  })
 })

 describe('FishLogo', () => {
--- a/packages/client/ui-primitives/tests/tooltip.spec.tsx
+++ b/packages/client/ui-primitives/tests/tooltip.spec.tsx
@@ -92,6 +92,37 @@ describe('Tooltip', () => {
    }
  })

+  it('reclamps after label and viewport width changes', () => {
+    const originalWidth = window.innerWidth
+    const spy = vi.spyOn(Element.prototype, 'getBoundingClientRect').mockImplementation(function (this: Element) {
+      if (this.getAttribute('role') !== 'tooltip') return rect(900, 1000)
+      return this.textContent === 'Wide' ? rect(900, 1100) : rect(850, 950)
+    })
+    try {
+      const view = render(
+        <Tooltip label="Wide" side="bottom">
+          <button type="button">anchor</button>
+        </Tooltip>,
+      )
+      fireEvent.mouseEnter(screen.getByText('anchor'))
+      expect(screen.getByRole('tooltip').style.left).toBe('862px')
+
+      view.rerender(
+        <Tooltip label="Short" side="bottom">
+          <button type="button">anchor</button>
+        </Tooltip>,
+      )
+      expect(screen.getByRole('tooltip').style.left).toBe('950px')
+
+      Object.defineProperty(window, 'innerWidth', { configurable: true, value: 900 })
+      fireEvent(window, new Event('resize'))
+      expect(screen.getByRole('tooltip').style.left).toBe('888px')
+    } finally {
+      Object.defineProperty(window, 'innerWidth', { configurable: true, value: originalWidth })
+      spy.mockRestore()
+    }
+  })
+
  it('clamps a bubble past the left viewport edge back inside', () => {
    const spy = vi.spyOn(Element.prototype, 'getBoundingClientRect').mockReturnValue(rect(-20, 80))
    try {