refactor(player/subtitle): comprehensive subtitle overlay improvements#226
Merged
Conversation
…h hook - Updated SubtitleContent component to use fixed line-height and color values. - Modified text handling in OriginalTextLine and TranslatedTextLine for better word wrapping and alignment. - Added useContentWidth hook to manage dynamic width styling for subtitles, improving layout consistency. - Adjusted SubtitleOverlay to utilize new width styles for better responsiveness in non-mask mode.
- Enhanced the calculateDragBounds function to ensure that subtitle components remain fully within the mask area by calculating absolute dimensions based on the mask viewport. - Introduced logic to prevent overflow of the right and bottom edges of the mask, improving the overall layout and user experience in mask mode.
…mode - Change resize handle visibility condition to require both mask mode and hover state - Reduce visual clutter by hiding resize handle when not actively interacting
- Render non-clickable tokens as plain spans instead of WordToken components - Only apply selection/hover states to clickable tokens - Simplify WordToken styling by removing conditional clickable styles - Improve performance by reducing unnecessary component rendering
- Move selected text state from UI hook to player store for global access - Add setSelectedText action in player store to update selected text - Remove local selectedText state from SubtitleOverlayUI hook - Enhance text selection logic with additional logging for debugging - Clear selected text on mouse leave and outside click events - Maintain backward compatibility with existing onTextSelection callback
…transform issues - Fix toast disappearing issue: add toast type differentiation to prevent mask-onboarding logic from interfering with copy success toast - Fix syntax errors: incomplete variable declaration and undefined variable references in coordinateTransform.ts - Clean up logging: remove duplicate logger.debug calls and unused variable references
Contributor
|
Caution Review failedThe pull request is closed. Walkthrough将字幕覆盖层重构为基于新 hooks 的模块化架构：新增坐标变换与 mask 视口、拖拽/缩放 hooks、移入 PlayerStore 的 selectedText、新增 SubtitleResizeHandle 与 SubtitleToast、调整词典弹窗与字幕词元渲染，并移除相关测试文件。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Content as SubtitleContent
participant OverlayHook as useSubtitleOverlay
participant Store as PlayerStore
participant Dict as DictionaryPopover
User->>Content: 点击/选择 单词
Content->>OverlayHook: 调用 setSelectedText(text)
OverlayHook->>Store: setSelectedText(text)
Store-->>OverlayHook: selectedText 更新
OverlayHook-->>Content: selectedText 可用
Content->>Dict: 条件性显示 字典弹窗
User->>Content: 点击容器外 / mouseleave
Content->>OverlayHook: 清除选择
OverlayHook->>Store: setSelectedText('')
Store-->>Content: 选择已清空
sequenceDiagram
participant User
participant Overlay as SubtitleOverlay
participant Drag as useSubtitleDrag
participant Resize as useSubtitleResize
participant Transform as coordinateTransform
participant Store as PlayerStore
User->>Overlay: 鼠标按下并拖拽
Overlay->>Drag: handleMouseDown
Drag->>Transform: 计算拖拽边界
Drag->>Store: onPositionChange(updatedPos)
Store-->>Overlay: 渲染新位置
User->>Overlay: 在把手上按下/拖拽
Overlay->>Resize: handleResizeMouseDown
Resize->>Transform: 计算 resize 边界
Resize->>Store: onSizeChange(updatedSize)
Store-->>Overlay: 渲染新尺寸
User->>Overlay: 双击把手
Resize->>Resize: 展开到最大并居中
Resize->>Store: 更新 size/position
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/renderer/src/pages/player/components/index.ts (1)
21-21: Electron 拖拽区域属性值应为 no-drag-webkit-app-region 仅支持 drag/no-drag；none 为无效值，可能导致标题栏区域拖拽/点击行为异常。
建议修改：
- -webkit-app-region: none; + -webkit-app-region: no-drag;src/renderer/src/pages/player/hooks/usePlayerShortcuts.ts (1)
11-11: logger context 命名与文件不符withContext('TransportBar') 与实际 Hook 名称不一致，影响日志定位。
-const logger = loggerService.withContext('TransportBar') +const logger = loggerService.withContext('PlayerShortcuts')src/renderer/src/pages/player/components/SubtitleContent.tsx (1)
599-624: 文本换行策略在中日韩语言下可能不佳OriginalTextLine/TranslatedTextLine/TokenizedTextContainer 同时使用了
white-space: pre-wrap+word-break: keep-all。对中文长句可能减少可断行位置，依赖overflow-wrap: break-word才会强制断行，体验上可能出现“整段先不换行，最后硬断”的跳跃。建议在 CJK 内容占比高的场景考虑
word-break: break-word或overflow-wrap: anywhere作降级，以减少突兀换行。可按模式或语言环境切换。Also applies to: 685-692
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
src/renderer/src/pages/player/components/SubtitleContent.tsx(15 hunks)src/renderer/src/pages/player/components/SubtitleOverlay.tsx(9 hunks)src/renderer/src/pages/player/components/SubtitleResizeHandle.tsx(1 hunks)src/renderer/src/pages/player/components/SubtitleToast.tsx(1 hunks)src/renderer/src/pages/player/components/index.ts(1 hunks)src/renderer/src/pages/player/hooks/index.ts(1 hunks)src/renderer/src/pages/player/hooks/useContainerBounds.ts(1 hunks)src/renderer/src/pages/player/hooks/useContentWidth.ts(1 hunks)src/renderer/src/pages/player/hooks/useMaskViewport.ts(1 hunks)src/renderer/src/pages/player/hooks/usePlayerShortcuts.ts(3 hunks)src/renderer/src/pages/player/hooks/useSubtitleDrag.ts(1 hunks)src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts(3 hunks)src/renderer/src/pages/player/hooks/useSubtitleOverlayUI.ts(2 hunks)src/renderer/src/pages/player/hooks/useSubtitleResize.ts(1 hunks)src/renderer/src/pages/player/utils/coordinateTransform.ts(1 hunks)src/renderer/src/pages/player/utils/index.ts(1 hunks)src/renderer/src/state/stores/player.store.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: 统一使用 loggerService 记录日志而不是 console
logger 错误记录示例需遵循第二参数为对象字面量：logger.error('Error in MediaClock listener:', { error: error })
Files:
src/renderer/src/pages/player/hooks/useSubtitleOverlayUI.tssrc/renderer/src/pages/player/hooks/useContentWidth.tssrc/renderer/src/pages/player/components/SubtitleToast.tsxsrc/renderer/src/pages/player/hooks/index.tssrc/renderer/src/pages/player/hooks/useSubtitleDrag.tssrc/renderer/src/pages/player/components/SubtitleOverlay.tsxsrc/renderer/src/pages/player/utils/coordinateTransform.tssrc/renderer/src/pages/player/utils/index.tssrc/renderer/src/pages/player/hooks/useSubtitleResize.tssrc/renderer/src/pages/player/components/SubtitleResizeHandle.tsxsrc/renderer/src/pages/player/components/index.tssrc/renderer/src/pages/player/hooks/usePlayerShortcuts.tssrc/renderer/src/pages/player/components/SubtitleContent.tsxsrc/renderer/src/state/stores/player.store.tssrc/renderer/src/pages/player/hooks/useContainerBounds.tssrc/renderer/src/pages/player/hooks/useMaskViewport.tssrc/renderer/src/pages/player/hooks/useSubtitleOverlay.ts
**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.tsx: 项目中所有图标统一使用 lucide-react，而非 emoji
优先使用 antd 组件库；如可被 antd 复用则优先选用 antd 而非自研
Files:
src/renderer/src/pages/player/components/SubtitleToast.tsxsrc/renderer/src/pages/player/components/SubtitleOverlay.tsxsrc/renderer/src/pages/player/components/SubtitleResizeHandle.tsxsrc/renderer/src/pages/player/components/SubtitleContent.tsx
🧬 Code graph analysis (12)
src/renderer/src/pages/player/hooks/useContentWidth.ts (1)
src/renderer/src/pages/player/hooks/index.ts (1)
useContentWidth(2-2)
src/renderer/src/pages/player/components/SubtitleToast.tsx (1)
src/renderer/src/infrastructure/styles/theme.ts (9)
ANIMATION_DURATION(91-100)EASING(103-114)Z_INDEX(75-88)GLASS_EFFECT(129-161)SPACING(43-58)BORDER_RADIUS(61-72)FONT_SIZES(25-40)FONT_WEIGHTS(11-22)SHADOWS(117-126)
src/renderer/src/pages/player/hooks/useSubtitleDrag.ts (2)
src/renderer/src/services/Logger.ts (1)
loggerService(817-817)src/renderer/src/pages/player/utils/coordinateTransform.ts (8)
Position(24-27)Size(29-32)MaskLayout(34-37)MAX_ESTIMATED_HEIGHT_PERCENT(18-18)ESTIMATED_SUBTITLE_HEIGHT_PX(17-17)calculateDragBounds(113-162)clampPercent(44-45)toMaskRelativePosition(50-58)
src/renderer/src/pages/player/components/SubtitleOverlay.tsx (9)
src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts (1)
useSubtitleOverlay(45-213)src/renderer/src/pages/player/hooks/useContentWidth.ts (1)
useContentWidth(21-33)src/renderer/src/pages/player/hooks/useMaskViewport.ts (1)
useMaskViewport(43-269)src/renderer/src/pages/player/utils/coordinateTransform.ts (6)
toMaskRelativePosition(50-58)toMaskRelativeSize(73-81)fromMaskRelativePosition(63-68)fromMaskRelativeSize(86-91)Position(24-27)Size(29-32)src/renderer/src/pages/player/hooks/useContainerBounds.ts (1)
useContainerBounds(30-165)src/renderer/src/pages/player/hooks/useSubtitleDrag.ts (1)
useSubtitleDrag(44-211)src/renderer/src/pages/player/hooks/useSubtitleResize.ts (1)
useSubtitleResize(45-282)src/renderer/src/pages/player/components/SubtitleResizeHandle.tsx (1)
SubtitleResizeHandle(34-63)src/renderer/src/pages/player/components/SubtitleToast.tsx (1)
SubtitleToast(38-48)
src/renderer/src/pages/player/utils/coordinateTransform.ts (2)
src/renderer/src/services/Logger.ts (1)
loggerService(817-817)src/renderer/src/pages/player/utils/index.ts (17)
MIN_SPAN_PERCENT(13-13)ESTIMATED_SUBTITLE_HEIGHT_PX(6-6)MAX_ESTIMATED_HEIGHT_PERCENT(10-10)MAX_OVERLAY_WIDTH_PERCENT(12-12)MAX_OVERLAY_HEIGHT_PERCENT_NORMAL_MODE(11-11)VIEWPORT_CHANGE_THRESHOLD(16-16)Position(1-1)Size(1-1)MaskLayout(1-1)clampPercent(5-5)toMaskRelativePosition(14-14)fromMaskRelativePosition(7-7)toMaskRelativeSize(15-15)fromMaskRelativeSize(8-8)hasViewportChangedSignificantly(9-9)calculateDragBounds(3-3)calculateResizeBounds(4-4)
src/renderer/src/pages/player/hooks/useSubtitleResize.ts (2)
src/renderer/src/services/Logger.ts (1)
loggerService(817-817)src/renderer/src/pages/player/utils/coordinateTransform.ts (9)
Position(24-27)Size(29-32)MaskLayout(34-37)calculateResizeBounds(167-195)MIN_SPAN_PERCENT(16-16)clampPercent(44-45)toMaskRelativeSize(73-81)MAX_OVERLAY_WIDTH_PERCENT(19-19)toMaskRelativePosition(50-58)
src/renderer/src/pages/player/components/SubtitleResizeHandle.tsx (1)
src/renderer/src/infrastructure/styles/theme.ts (2)
ANIMATION_DURATION(91-100)EASING(103-114)
src/renderer/src/pages/player/hooks/usePlayerShortcuts.ts (1)
src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts (1)
useSubtitleOverlay(45-213)
src/renderer/src/pages/player/components/SubtitleContent.tsx (3)
src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts (1)
useSubtitleOverlay(45-213)src/renderer/src/utils/textTokenizer.ts (2)
isClickableToken(134-136)WordToken(8-19)src/renderer/src/pages/player/components/DictionaryPopover.tsx (1)
DictionaryPopover(45-245)
src/renderer/src/pages/player/hooks/useContainerBounds.ts (1)
src/renderer/src/pages/player/hooks/index.ts (1)
useContainerBounds(1-1)
src/renderer/src/pages/player/hooks/useMaskViewport.ts (1)
src/renderer/src/pages/player/utils/coordinateTransform.ts (6)
Position(24-27)Size(29-32)MaskLayout(34-37)clampPercent(44-45)MIN_SPAN_PERCENT(16-16)hasViewportChangedSignificantly(96-108)
src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts (1)
src/renderer/src/state/stores/player.store.ts (1)
usePlayerStore(735-737)
🪛 Biome (2.1.2)
src/renderer/src/pages/player/components/SubtitleToast.tsx
[error] 52-54: The elements with this role can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
🪛 GitHub Actions: Test
src/renderer/src/pages/player/hooks/useSubtitleResize.ts
[error] 112-112: TypeScript error TS6133: 'heightLimit' is declared but its value is never read. (Triggered during 'npm run typecheck:node' per 'tsc --noEmit -p tsconfig.node.json --composite false')
🪛 GitHub Check: test (macos-latest, 20)
src/renderer/src/pages/player/hooks/useSubtitleResize.ts
[failure] 112-112:
'heightLimit' is declared but its value is never read.
🪛 GitHub Check: test (ubuntu-latest, 20)
src/renderer/src/pages/player/hooks/useSubtitleResize.ts
[failure] 112-112:
'heightLimit' is declared but its value is never read.
🔇 Additional comments (8)
src/renderer/src/pages/player/hooks/useSubtitleOverlayUI.ts (1)
229-235: ✓ setSubtitleOverlay 已正确实现浅合并，无需修改验证结果显示
setSubtitleOverlay的实现（player.store.ts:566-568）已正确执行浅合并操作：const overlay = { ...s.subtitleOverlay, ...overlayConfig }该实现使用展开运算符正确地合并现有配置与新传入的部分字段，不存在整对象替换导致数据丢失的问题。代码实现已符合预期，无需进一步调整。
src/renderer/src/pages/player/utils/index.ts (1)
1-17: Barrel 汇总导出清晰可用统一从单入口导出坐标工具与类型，利于消费端解耦，LGTM。
src/renderer/src/pages/player/components/index.ts (1)
14-15: 新增组件导出 OKSubtitleResizeHandle 与 SubtitleToast 的公开导出符合本 PR 架构拆分，LGTM。
src/renderer/src/pages/player/hooks/usePlayerShortcuts.ts (1)
44-50: 复制优先使用选中文本的逻辑正确优先 selectedText 且做 trim，日志记录长度，流程合理，LGTM。
src/renderer/src/pages/player/hooks/index.ts (1)
1-3: Hook barrel 导出齐全新增 useContainerBounds/useContentWidth/useMaskViewport/useSubtitleDrag/useSubtitleResize 的统一导出合理，LGTM。
Also applies to: 5-5, 9-9
src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts (1)
24-27: 公共 API 扩展（selectedText/setSelectedText）需同步更新类型使用方返回类型变更会影响依赖此 Hook 的组件/Hook 的类型推断，请确认调用方已更新。
src/renderer/src/state/stores/player.store.ts (1)
169-171: 新增 runtime 状态 selectedText 合理作为非持久化运行时数据放在 PlayerState 初始值，符合用途，LGTM。
src/renderer/src/pages/player/components/SubtitleContent.tsx (1)
258-265: 选择与清除逻辑清晰，事件隔离到位，日志规范
- 选中范围、全局 click 清理、mouseleave 清理、字典弹窗阻止冒泡均处理完善。
- 日志使用 loggerService，结构化对象参数，符合规范。
Also applies to: 283-307, 331-357, 360-373
- update `onMouseDown` and `onDoubleClick` to use `React.MouseEvent<HTMLDivElement>` - improve type safety by matching the actual element
…and redundant onTextSelection forwarding - Remove duplicate useSubtitleOverlay hook call in SubtitleOverlay.tsx - Remove unnecessary handleTextSelection callback and onTextSelection prop forwarding - Remove onTextSelection parameter from SubtitleContent interface and implementation - Simplify text selection state management by using only internal setSelectedText calls - Update dependency arrays in useCallback and useEffect hooks - Improve performance by reducing redundant store subscriptions
… hook - remove onModeChange callback from UseMaskViewportOptions - remove onModeChange parameter from hook implementation - eliminate previousMaskModeRef and mode change effect - clean up unused dependencies in hook implementation
- Add useEffect to clear selected text when subtitle index changes - Prevent stale text selection when new subtitle appears
- replace MAX_OVERLAY_WIDTH_PERCENT with MAX_OVERLAY_HEIGHT_PERCENT_NORMAL_MODE for heightLimit - ensure subtitle overlay height is properly limited to 40% in normal mode
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/renderer/src/pages/player/components/SubtitleOverlay.tsx (2)
241-256: 事件类型收窄与安全访问
addEventListener强转为EventListener，回调内部直接解构event.detail.message。建议做类型守卫，避免非预期事件导致运行时异常。- useEffect(() => { - const handleSubtitleCopied = (event: CustomEvent<{ message: string }>) => { - const { message } = event.detail - showToast(message, 'copy') - } + useEffect(() => { + const handleSubtitleCopied = (evt: Event) => { + const e = evt as CustomEvent<{ message?: string }> + const message = e?.detail?.message + if (typeof message === 'string' && message.length > 0) { + showToast(message, 'copy') + } + } @@ - window.addEventListener('subtitle-copied', handleSubtitleCopied as EventListener) + window.addEventListener('subtitle-copied', handleSubtitleCopied as EventListener) @@ - window.removeEventListener('subtitle-copied', handleSubtitleCopied as EventListener) + window.removeEventListener('subtitle-copied', handleSubtitleCopied as EventListener)
409-409: 收窄过渡属性，减少不必要的重绘/回流
transition: all容易放大开销并引入意外过渡。建议只过渡必要属性。- transition: all 200ms ease-out; + transition: + top 200ms ease-out, + left 200ms ease-out, + width 200ms ease-out, + height 200ms ease-out, + transform 200ms ease-out, + border-color 200ms ease-out, + box-shadow 200ms ease-out;
♻️ Duplicate comments (3)
src/renderer/src/pages/player/utils/coordinateTransform.ts (1)
141-151: 收敛调试日志字段并避免在生产环境冗余输出当前 debug 日志包含大量字段（maskViewport/currentSize/effectiveXY 等），建议仅保留关键值并在生产禁用或按级别过滤，降低噪音与序列化开销。此问题已在先前评论提出，仍建议调整。
参考精简：
- logger.debug('Drag bounds calculated:', { - isMaskMode, - maskViewport, - currentSize, - effectiveWidth, - effectiveHeight, - maskRight, - maskBottom, - bounds - }) + if (process.env.NODE_ENV !== 'production') { + logger.debug('drag-bounds', { + effectiveSize: { w: Math.round(effectiveWidth * 100) / 100, h: Math.round(effectiveHeight * 100) / 100 }, + bounds + }) + }src/renderer/src/pages/player/hooks/useMaskViewport.ts (1)
234-239: 返回占位的 overlayPosition/overlaySize 容易误用；建议移除或计算真实值当前始终返回
{ x:0,y:0 }/{ width:0,height:0 }，调用方若误信为真实值会产生布局异常。此前已有类似提醒，建议将其从返回类型移除，由上层统一计算。建议修改接口与返回：
export interface UseMaskViewportReturn { maskViewport: MaskLayout | null - overlayPosition: Position - overlaySize: Size videoAspectRatio: number | null } ... return { maskViewport, - overlayPosition: { x: 0, y: 0 }, // 将在主组件中计算 - overlaySize: { width: 0, height: 0 }, // 将在主组件中计算 videoAspectRatio }如需保留，请改为
undefined或计算真实值，避免“看似有效”的零值。src/renderer/src/pages/player/components/SubtitleResizeHandle.tsx (1)
43-61: 保持常驻渲染以支持淡出；修正光标方向/过渡范围并在隐藏时禁用指针事件
visible=false时直接return null，将丢失淡出动画。- 句柄位于右下角，应使用
se-resize/nwse-resize。transition: all过重；仅过渡opacity/transform。- 隐藏时应
pointer-events: none，避免透明层拦截事件。建议合并修复：
@@ - if (!visible) { - return null - } + // 常驻渲染，使用样式控制可见性与交互性 @@ const ResizeHandle = styled.div<{ $visible: boolean }>` @@ - cursor: nw-resize; + cursor: se-resize; /* 或 nwse-resize */ - opacity: ${(props) => (props.$visible ? 1 : 0)}; - transition: all ${ANIMATION_DURATION.MEDIUM} ${EASING.STANDARD}; + opacity: ${(props) => (props.$visible ? 1 : 0)}; + pointer-events: ${(props) => (props.$visible ? 'auto' : 'none')}; + transition: + opacity ${ANIMATION_DURATION.MEDIUM} ${EASING.STANDARD}, + transform ${ANIMATION_DURATION.MEDIUM} ${EASING.STANDARD};Also applies to: 69-96
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
src/renderer/src/pages/player/components/SubtitleContent.tsx(15 hunks)src/renderer/src/pages/player/components/SubtitleOverlay.tsx(9 hunks)src/renderer/src/pages/player/components/SubtitleResizeHandle.tsx(1 hunks)src/renderer/src/pages/player/hooks/useMaskViewport.ts(1 hunks)src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts(5 hunks)src/renderer/src/pages/player/utils/coordinateTransform.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: 统一使用 loggerService 记录日志而不是 console
logger 错误记录示例需遵循第二参数为对象字面量：logger.error('Error in MediaClock listener:', { error: error })
Files:
src/renderer/src/pages/player/utils/coordinateTransform.tssrc/renderer/src/pages/player/components/SubtitleContent.tsxsrc/renderer/src/pages/player/hooks/useSubtitleOverlay.tssrc/renderer/src/pages/player/components/SubtitleResizeHandle.tsxsrc/renderer/src/pages/player/components/SubtitleOverlay.tsxsrc/renderer/src/pages/player/hooks/useMaskViewport.ts
**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.tsx: 项目中所有图标统一使用 lucide-react，而非 emoji
优先使用 antd 组件库；如可被 antd 复用则优先选用 antd 而非自研
Files:
src/renderer/src/pages/player/components/SubtitleContent.tsxsrc/renderer/src/pages/player/components/SubtitleResizeHandle.tsxsrc/renderer/src/pages/player/components/SubtitleOverlay.tsx
🧬 Code graph analysis (6)
src/renderer/src/pages/player/utils/coordinateTransform.ts (1)
src/renderer/src/services/Logger.ts (1)
loggerService(817-817)
src/renderer/src/pages/player/components/SubtitleContent.tsx (2)
src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts (1)
useSubtitleOverlay(45-221)src/renderer/src/utils/textTokenizer.ts (2)
isClickableToken(134-136)WordToken(8-19)
src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts (1)
src/renderer/src/state/stores/player.store.ts (1)
usePlayerStore(735-737)
src/renderer/src/pages/player/components/SubtitleResizeHandle.tsx (1)
src/renderer/src/infrastructure/styles/theme.ts (2)
ANIMATION_DURATION(91-100)EASING(103-114)
src/renderer/src/pages/player/components/SubtitleOverlay.tsx (8)
src/renderer/src/pages/player/hooks/useContentWidth.ts (1)
useContentWidth(21-33)src/renderer/src/pages/player/hooks/useMaskViewport.ts (1)
useMaskViewport(35-240)src/renderer/src/pages/player/utils/coordinateTransform.ts (6)
toMaskRelativePosition(50-58)toMaskRelativeSize(73-81)fromMaskRelativePosition(63-68)fromMaskRelativeSize(86-91)Position(24-27)Size(29-32)src/renderer/src/pages/player/hooks/useContainerBounds.ts (1)
useContainerBounds(30-165)src/renderer/src/pages/player/hooks/useSubtitleDrag.ts (1)
useSubtitleDrag(44-211)src/renderer/src/pages/player/hooks/useSubtitleResize.ts (1)
useSubtitleResize(45-281)src/renderer/src/pages/player/components/SubtitleResizeHandle.tsx (1)
SubtitleResizeHandle(34-63)src/renderer/src/pages/player/components/SubtitleToast.tsx (1)
SubtitleToast(38-48)
src/renderer/src/pages/player/hooks/useMaskViewport.ts (1)
src/renderer/src/pages/player/utils/coordinateTransform.ts (6)
MaskLayout(34-37)Position(24-27)Size(29-32)clampPercent(44-45)MIN_SPAN_PERCENT(16-16)hasViewportChangedSignificantly(96-108)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test (ubuntu-latest, 20)
- GitHub Check: test (macos-latest, 20)
- GitHub Check: test (windows-latest, 20)
🔇 Additional comments (4)
src/renderer/src/pages/player/utils/coordinateTransform.ts (2)
190-195: 常量选择正确，已修复先前“高度限制误用”问题
heightLimit/maxHeightPercent均使用MAX_OVERLAY_HEIGHT_PERCENT_NORMAL_MODE，与需求一致。
130-139: 审查意见不准确 - 代码无需修改经过验证，原审查意见所指出的问题不存在。代码实际上是正确的，无需进行建议的修改。
验证依据：
坐标系统统一：在
useSubtitleResize.ts中，maskViewport.size.width与MAX_OVERLAY_WIDTH_PERCENT（值为 95）直接比较，证明maskViewport使用容器百分比单位。currentSize 单位一致：
- 非遮罩模式（第 158 行）：
xMax = 100 - currentSize.width说明currentSize.width是百分比- 遮罩模式使用同一变量，因此
currentSize已经是容器百分比，无需转换计算逻辑有效：遮罩模式下
xMax = maskRight - effectiveWidth的计算在百分比坐标系中完全正确。例如：遮罩位置 x=10%、宽度 80%（范围 1090），覆盖层宽度 20%，则 xMax=70% 表示覆盖层可在 1070% 范围移动，确保不超出遮罩边界。注释与代码的关系：第 130 行的注释措辞确实容易误导（"转换为相对容器的绝对尺寸"），但代码无需修改——所有值已使用一致的容器百分比单位。
Likely an incorrect or invalid review comment.
src/renderer/src/pages/player/components/SubtitleContent.tsx (1)
321-359: 清除选中逻辑完善且与全局事件解绑配合良好全局点击与鼠标离开时均同步重置组件状态与 store 中的
selectedText，保证 UI 与状态一致性。src/renderer/src/pages/player/components/SubtitleOverlay.tsx (1)
55-57: 移除重复状态接入点，已解决过往重复订阅问题这里仅保留了一次
useSubtitleOverlay()，且不再向SubtitleContent转发onTextSelection，与先前审查意见一致。实现更简洁，也避免二次写入 store。
Comment on lines
+280
to
293
| logger.debug('准备设置选中文本', { | ||
| minIndex, | ||
| maxIndex, | ||
| selectedTokensCount: selectedTokens.length, | ||
| selectedText: `"${selectedText}"` | ||
| }) | ||
|
|
||
| if (selectedText.trim()) { | ||
| setSelectedText(selectedText) | ||
| logger.info('划词选中文本', { | ||
| selectedText: selectedText.substring(0, 50) + (selectedText.length > 50 ? '...' : ''), | ||
| length: selectedText.length, | ||
| tokenCount: selectedTokens.length | ||
| }) |
Contributor
There was a problem hiding this comment.
避免记录选中文本原文，降低隐私/合规风险
日志中包含截断后的 selectedText 片段，仍可能泄露用户内容。建议仅记录长度与索引范围。
应用精简：
- if (selectedText.trim()) {
- setSelectedText(selectedText)
- logger.info('划词选中文本', {
- selectedText: selectedText.substring(0, 50) + (selectedText.length > 50 ? '...' : ''),
- length: selectedText.length,
- tokenCount: selectedTokens.length
- })
+ if (selectedText.trim()) {
+ setSelectedText(selectedText)
+ logger.info('划词选中文本', {
+ length: selectedText.length,
+ tokenCount: selectedTokens.length,
+ range: { from: minIndex, to: maxIndex }
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| logger.debug('准备设置选中文本', { | |
| minIndex, | |
| maxIndex, | |
| selectedTokensCount: selectedTokens.length, | |
| selectedText: `"${selectedText}"` | |
| }) | |
| if (selectedText.trim()) { | |
| setSelectedText(selectedText) | |
| logger.info('划词选中文本', { | |
| selectedText: selectedText.substring(0, 50) + (selectedText.length > 50 ? '...' : ''), | |
| length: selectedText.length, | |
| tokenCount: selectedTokens.length | |
| }) | |
| logger.debug('准备设置选中文本', { | |
| minIndex, | |
| maxIndex, | |
| selectedTokensCount: selectedTokens.length, | |
| selectedText: `"${selectedText}"` | |
| }) | |
| if (selectedText.trim()) { | |
| setSelectedText(selectedText) | |
| logger.info('划词选中文本', { | |
| length: selectedText.length, | |
| tokenCount: selectedTokens.length, | |
| range: { from: minIndex, to: maxIndex } | |
| }) |
🤖 Prompt for AI Agents
In src/renderer/src/pages/player/components/SubtitleContent.tsx around lines 280
to 293, the logs currently include the raw or truncated selectedText which can
leak user content; remove any selectedText values from logger.debug and
logger.info and instead log only non-sensitive metadata (e.g.,
selectedTokensCount, selection length, minIndex, maxIndex). Update the debug
call to omit selectedText and keep minIndex/maxIndex/selectedTokensCount, and
change the info call to log length, tokenCount, minIndex and maxIndex (no text
snippets), preserving the existing conditional setSelectedText behavior.
Comment on lines
+160
to
+179
| const { handleMouseDown: handleDragMouseDown, updateLatestPosition } = useSubtitleDrag({ | ||
| overlayPosition, | ||
| overlaySize, | ||
| containerBounds, | ||
| isMaskMode, | ||
| maskViewport, | ||
| isResizing, | ||
| autoPositioning, | ||
| onPositionChange: setPosition, | ||
| onDragStart: startDragging, | ||
| onDragEnd: stopDragging, | ||
| onDisableAutoPositioning: () => { | ||
| usePlayerStore.getState().setSubtitleOverlay({ autoPositioning: false }) | ||
| }, | ||
| onMaskOnboardingComplete: () => { | ||
| setMaskOnboardingComplete(true) | ||
| setToastVisible(false) | ||
| }, | ||
| overlayRef | ||
| }) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
store 更新路径建议统一到 integration（一致性与可测性）
onDisableAutoPositioning 通过 usePlayerStore.getState().setSubtitleOverlay 直接改 store。为保持与 setPosition / setSize 一致，可以在 useSubtitleOverlay() 暴露 setAutoPositioning(false)，由此组件仅依赖 integration 层，便于替换/mock。
是否需要我在 hooks 层补一个 setAutoPositioning 并在此处替换？
Also applies to: 182-206
🤖 Prompt for AI Agents
In src/renderer/src/pages/player/components/SubtitleOverlay.tsx around lines
160-179 (and similarly 182-206), the component directly calls
usePlayerStore.getState().setSubtitleOverlay(...) to disable auto-positioning;
instead add and expose a setAutoPositioning(false) function from the
useSubtitleOverlay (integration) hook and call that here. Modify the hook to
accept/dispatch an action that toggles autoPositioning in the store (or updates
local overlay state) and replace the direct store mutation in this file with the
exported setAutoPositioning(false) so the component only depends on the
integration layer for state changes.
Comment on lines
+316
to
+318
| $width={isMaskMode ? 'none' : widthStyle} | ||
| $maxWidth={isMaskMode ? 'none' : maxWidthStyle} | ||
| onMouseDown={handleDragMouseDown} |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
避免向样式层传递潜在无效的 CSS 值
虽然遮罩模式下 $width 分支不会被使用，但传入 'none' 对 width 是无效值。为稳妥起见，建议传 auto。
- $width={isMaskMode ? 'none' : widthStyle}
+ $width={isMaskMode ? 'auto' : widthStyle}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $width={isMaskMode ? 'none' : widthStyle} | |
| $maxWidth={isMaskMode ? 'none' : maxWidthStyle} | |
| onMouseDown={handleDragMouseDown} | |
| $width={isMaskMode ? 'auto' : widthStyle} | |
| $maxWidth={isMaskMode ? 'none' : maxWidthStyle} | |
| onMouseDown={handleDragMouseDown} |
🤖 Prompt for AI Agents
In src/renderer/src/pages/player/components/SubtitleOverlay.tsx around lines 316
to 318, the styled props pass the string 'none' for $width and $maxWidth when
isMaskMode; replace those ternary branches to return 'auto' instead of 'none' so
the component never supplies an invalid CSS value for width/max-width, and
ensure any typings accept the 'auto' string.
Comment on lines
+338
to
+343
| {/* 只在遮罩模式下且悬停时显示调整大小句柄 */} | ||
| <SubtitleResizeHandle | ||
| visible={isMaskMode && isHovered} | ||
| onMouseDown={handleResizeMouseDown} | ||
| onDoubleClick={handleResizeDoubleClick} | ||
| /> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
遮罩模式仅靠 hover 显示尺寸柄，缺少键盘可达性
目前 SubtitleResizeHandle 只在 isHovered 为 true 时可见。键盘用户（Tab 聚焦到 Overlay）无法看到/使用尺寸柄。建议在 Overlay 获得焦点时同样显示尺寸柄，提升可达性。
应用以下增量修改：
- 增加焦点状态与处理函数
@@
- const [toastType, setToastType] = useState<'copy' | 'mask-onboarding' | null>(null)
+ const [toastType, setToastType] = useState<'copy' | 'mask-onboarding' | null>(null)
+ const [isFocused, setIsFocused] = useState(false)
@@
const handleMouseLeave = useCallback(() => {
if (!isDragging && !isResizing) {
setHovered(false)
}
}, [setHovered, isDragging, isResizing])
+
+ const handleFocus = useCallback(() => setIsFocused(true), [])
+ const handleBlur = useCallback(() => setIsFocused(false), [])- 绑定 onFocus/onBlur，并在可见性判断中加入
isFocused
@@
<OverlayContainer
@@
onMouseEnter={handleMouseEnter}
onMouseLeave={handleMouseLeave}
+ onFocus={handleFocus}
+ onBlur={handleBlur}
@@
- <SubtitleResizeHandle
- visible={isMaskMode && isHovered}
+ <SubtitleResizeHandle
+ visible={isMaskMode && (isHovered || isFocused)}
onMouseDown={handleResizeMouseDown}
onDoubleClick={handleResizeDoubleClick}
/>验证：使用 Tab 聚焦 Overlay（已有 tabIndex={0}），应可见尺寸柄并可用鼠标执行拖拽/双击行为。
Also applies to: 275-285, 90-96
我需要验证该review comment中的建议。让我查看文件和代码上下文。
正在验证review comment中的可达性问题...
🏁 Script executed:
#!/bin/bash
# Locate and display SubtitleOverlay.tsx content
fd -t f "SubtitleOverlay.tsx" -x cat {}Length of output: 13676
🏁 Script executed:
#!/bin/bash
# Search for isFocused state and focus handlers
rg -n "isFocused|handleFocus|handleBlur|onFocus|onBlur" src/renderer/src/pages/player/components/SubtitleOverlay.tsx -A 2 -B 2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check for OverlayContainer element and its props around lines 338-343
rg -n "OverlayContainer" src/renderer/src/pages/player/components/SubtitleOverlay.tsx -A 10 -B 5Length of output: 1201
🏁 Script executed:
#!/bin/bash
# Check lines around 90-96 and 275-285 for context
cat -n src/renderer/src/pages/player/components/SubtitleOverlay.tsx | sed -n '85,100p;270,290p'Length of output: 1514
🏁 Script executed:
#!/bin/bash
# Search for SubtitleResizeHandle visible prop condition
rg -n "SubtitleResizeHandle" src/renderer/src/pages/player/components/SubtitleOverlay.tsx -A 5 -B 2Length of output: 546
🏁 Script executed:
#!/bin/bash
# Check coding guidelines - verify logger usage if any logging exists
rg -n "console\." src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check for emoji usage in this file
rg -n "[😀-🙏🌀-🗿🚀-]" src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 0
🏁 Script executed:
#!/bin/bash
# Search for useState and useCallback patterns to verify current implementation
rg -n "useState|useCallback" src/renderer/src/pages/player/components/SubtitleOverlay.tsx | head -20Length of output: 570
🏁 Script executed:
#!/bin/bash
# Check what icon library is being used
rg -n "import.*from.*icon|from.*lucide" src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 0
🏁 Script executed:
#!/bin/bash
# Get full context around line 338-343 with proper line numbers
sed -n '330,350p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx | cat -nLength of output: 786
🏁 Script executed:
#!/bin/bash
# Check for tabIndex on OverlayContainer
rg -n "tabIndex" src/renderer/src/pages/player/components/SubtitleOverlay.tsx -B 5 -A 2Length of output: 262
🏁 Script executed:
#!/bin/bash
# Search for handleMouseEnter and handleMouseLeave to understand current hover handling
rg -n "handleMouse|setHovered" src/renderer/src/pages/player/components/SubtitleOverlay.tsx -A 3 -B 1Length of output: 965
🏁 Script executed:
#!/bin/bash
# Get the toastType state declaration to see where to add isFocused
rg -n "toastType" src/renderer/src/pages/player/components/SubtitleOverlay.tsx -A 5 -B 2 | head -15Length of output: 697
🏁 Script executed:
#!/bin/bash
# Check the full visible condition for SubtitleResizeHandle
rg -n "visible=\{" src/renderer/src/pages/player/components/SubtitleOverlay.tsx -A 1 -B 1Length of output: 245
🏁 Script executed:
#!/bin/bash
# Examine component's exported type if it's a functional component
rg -n "export.*SubtitleOverlay|function SubtitleOverlay|const SubtitleOverlay" src/renderer/src/pages/player/components/SubtitleOverlay.tsx -A 5Length of output: 484
🏁 Script executed:
#!/bin/bash
# Check for any existing focus/blur event listeners on OverlayContainer
grep -n "onFocus\|onBlur\|onMouseEnter\|onMouseLeave" src/renderer/src/pages/player/components/SubtitleOverlay.tsx | head -15Length of output: 87
🏁 Script executed:
#!/bin/bash
# Verify the exact structure around visible prop for SubtitleResizeHandle
sed -n '335,345p' src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 331
🏁 Script executed:
#!/bin/bash
# Check for isHovered state and its usage
rg -n "isHovered|setHovered\|isMaskMode" src/renderer/src/pages/player/components/SubtitleOverlay.tsx -B 2 -A 2Length of output: 879
🏁 Script executed:
#!/bin/bash
# Full component structure search - look for return statement and JSX
rg -n "return\s*\(" src/renderer/src/pages/player/components/SubtitleOverlay.tsx -A 50 | head -80Length of output: 2753
🏁 Script executed:
#!/bin/bash
# Get surrounding context for lines 275-285
sed -n '270,290p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx | cat -nLength of output: 689
🏁 Script executed:
#!/bin/bash
# Get surrounding context for lines 90-96
sed -n '85,100p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx | cat -nLength of output: 824
🏁 Script executed:
#!/bin/bash
# Check if useState or state variables are used before line 100
sed -n '1,110p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx | grep -n "useState\|const \[" | head -20Length of output: 297
🏁 Script executed:
#!/bin/bash
# Check all event handlers on OverlayContainer - full structure
sed -n '330,350p' src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 639
🏁 Script executed:
#!/bin/bash
# Look for the word "keyboard" or "accessible" or "a11y" in file
rg -ni "keyboard|accessible|a11y|aria" src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 30
🏁 Script executed:
#!/bin/bash
# Check current visible condition for resize handle - get exact code
rg -n "visible=.*isMaskMode" src/renderer/src/pages/player/components/SubtitleOverlay.tsx -A 2 -B 2Length of output: 225
🏁 Script executed:
#!/bin/bash
# Full file size check to understand if lines exist
wc -l src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 64
🏁 Script executed:
#!/bin/bash
# Get complete OverlayContainer JSX element definition
rg -n "<OverlayContainer" src/renderer/src/pages/player/components/SubtitleOverlay.tsx -A 20 | head -40Length of output: 813
🏁 Script executed:
#!/bin/bash
# Find all useState declarations in the file
rg -n "useState<" src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 88
🏁 Script executed:
#!/bin/bash
# Find the handleMouseLeave function definition mentioned in the review
rg -n "const handleMouseLeave" src/renderer/src/pages/player/components/SubtitleOverlay.tsx -A 6Length of output: 251
🏁 Script executed:
#!/bin/bash
# Final comprehensive check - look at full state and handlers section
sed -n '1,150p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx | tail -100Length of output: 3061
🏁 Script executed:
#!/bin/bash
# Check specific lines exactly as mentioned in review
sed -n '90p;91p;92p;93p;94p;95p;96p;275p;276p;277p;278p;279p;280p;281p;282p;283p;284p;285p;338p;339p;340p;341p;342p;343p' src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 830
🏁 Script executed:
#!/bin/bash
# Search for the exact visible pattern mentioned in the diff
rg "visible=\{isMaskMode && isHovered\}" src/renderer/src/pages/player/components/SubtitleOverlay.tsx -nLength of output: 47
🏁 Script executed:
#!/bin/bash
# Check if isFocused already exists in file
grep -n "isFocused" src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 0
🏁 Script executed:
#!/bin/bash
# Final state - look at complete component return/render section
sed -n '320,360p' src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 1168
🏁 Script executed:
#!/bin/bash
# Verify all event binding points needed for focus handling
grep -n "onMouseEnter\|onMouseLeave\|onFocus\|onBlur" src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 87
🏁 Script executed:
#!/bin/bash
# Get accurate line count and verify file integrity
file src/renderer/src/pages/player/components/SubtitleOverlay.tsx && wc -l src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 42
🏁 Script executed:
#!/bin/bash
# Comprehensive state check - all useState calls
sed -n '1,200p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx | grep -n "useState"Length of output: 297
🏁 Script executed:
#!/bin/bash
# Check the exact context where toastType is declared to understand where isFocused should be added
sed -n '80,120p' src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 1420
🏁 Script executed:
#!/bin/bash
# Get the area around handleMouseLeave to verify current event handler pattern
sed -n '250,300p' src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 1508
🏁 Script executed:
#!/bin/bash
# Final verification - look for both isMaskMode and isHovered in context
grep -n "isMaskMode\|isHovered" src/renderer/src/pages/player/components/SubtitleOverlay.tsx | head -20Length of output: 963
🏁 Script executed:
#!/bin/bash
# Confirm SubtitleResizeHandle is being rendered with correct props
sed -n '338,350p' src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 331
🏁 Script executed:
#!/bin/bash
# Check if the issue is already fixed - search for isFocused in visible condition
rg "visible=.*isFocused" src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 0
🏁 Script executed:
#!/bin/bash
# Get exact line 343 to see what's there now
sed -n '343p' src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 10
🏁 Script executed:
#!/bin/bash
# Complete render section of component to verify implementation state
sed -n '315,350p' src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 1168
🏁 Script executed:
#!/bin/bash
# Verify keyboard event handling patterns used elsewhere in codebase
rg "handleFocus\|handleBlur" src/renderer/src/pages/player/components/ -lLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check imports and component structure
head -30 src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 907
🏁 Script executed:
#!/bin/bash
# Complete verification - show full file to analyze state and render structure
cat src/renderer/src/pages/player/components/SubtitleOverlay.tsx | wc -l && echo "---" && head -50 src/renderer/src/pages/player/components/SubtitleOverlay.tsx && echo "--- MID SECTION ---" && sed -n '80,130p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx && echo "--- EVENT HANDLERS ---" && sed -n '240,300p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx && echo "--- RENDER ---" && sed -n '320,350p' src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 6116
🏁 Script executed:
#!/bin/bash
# Determine exact file content and lines mentioned in review
echo "Total lines:" && wc -l src/renderer/src/pages/player/components/SubtitleOverlay.tsx && echo "" && echo "Lines 338-343:" && sed -n '338,343p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx && echo "" && echo "Lines 275-285:" && sed -n '275,285p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx && echo "" && echo "Lines 90-96:" && sed -n '90,96p' src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 954
🏁 Script executed:
#!/bin/bash
# Final check - search state declarations and focus event setup
sed -n '1,300p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx | grep -A 1 "useState\|handleFocus\|handleBlur\|onFocus\|onBlur"Length of output: 414
🏁 Script executed:
#!/bin/bash
# Just get the key sections cleanly
echo "=== FILE STATS ===" && wc -l src/renderer/src/pages/player/components/SubtitleOverlay.tsx && echo "" && echo "=== LINES 90-96 ===" && sed -n '90,96p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx && echo "" && echo "=== LINES 275-285 ===" && sed -n '275,285p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx && echo "" && echo "=== LINES 338-343 ===" && sed -n '338,343p' src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 981
🏁 Script executed:
#!/bin/bash
# Search isFocused state usage
grep -c "isFocused" src/renderer/src/pages/player/components/SubtitleOverlay.tsx 2>/dev/null && echo "isFocused found" || echo "isFocused NOT found in file"Length of output: 29
🏁 Script executed:
#!/bin/bash
# Search handleFocus usage
grep -c "handleFocus\|handleBlur" src/renderer/src/pages/player/components/SubtitleOverlay.tsx 2>/dev/null && echo "handleFocus/handleBlur found" || echo "handleFocus/handleBlur NOT found in file"Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check current visible condition
grep "visible=" src/renderer/src/pages/player/components/SubtitleOverlay.tsx | grep -i "resize\|handle"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Most concise final verification
echo "=== Current Implementation Check ===" && echo "isFocused state exists:" && (grep -q "isFocused" src/renderer/src/pages/player/components/SubtitleOverlay.tsx && echo "YES" || echo "NO") && echo "handleFocus handler exists:" && (grep -q "handleFocus" src/renderer/src/pages/player/components/SubtitleOverlay.tsx && echo "YES" || echo "NO") && echo "focus in SubtitleResizeHandle visible:" && (grep "SubtitleResizeHandle" src/renderer/src/pages/player/components/SubtitleOverlay.tsx | grep -q "isFocused" && echo "YES" || echo "NO")Length of output: 136
🏁 Script executed:
#!/bin/bash
# Display exact lines with context
awk 'NR>=88 && NR<=98 {print NR": "$0} NR>=273 && NR<=287 {print NR": "$0} NR>=336 && NR<=345 {print NR": "$0}' src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 1449
🏁 Script executed:
#!/bin/bash
# Show complete state area + render area
echo "=== STATE AREA (lines 80-110) ===" && sed -n '80,110p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx && echo "" && echo "=== EVENT HANDLERS (lines 270-290) ===" && sed -n '270,290p' src/renderer/src/pages/player/components/SubtitleOverlay.tsx && echo "" && echo "=== RENDER/RESIZE HANDLE (lines 335-345) ===" && sed -n '335,345p' src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 2097
🏁 Script executed:
#!/bin/bash
# Complete file extraction to pipe to analysis
cat src/renderer/src/pages/player/components/SubtitleOverlay.tsxLength of output: 13676
🏁 Script executed:
#!/bin/bash
# Simple direct check of exact section
cat src/renderer/src/pages/player/components/SubtitleOverlay.tsx | sed -n '338,343p'Length of output: 216
🏁 Script executed:
#!/bin/bash
# Show what's currently bound to OverlayContainer
cat src/renderer/src/pages/player/components/SubtitleOverlay.tsx | grep -A 15 "OverlayContainer"Length of output: 1061
🏁 Script executed:
#!/bin/bash
# Check if this is TypeScript and look for useState type declarations
cat src/renderer/src/pages/player/components/SubtitleOverlay.tsx | head -60 | tail -50Length of output: 1486
🏁 Script executed:
#!/bin/bash
# Direct validation - extract the three mentioned line ranges
head -n 343 src/renderer/src/pages/player/components/SubtitleOverlay.tsx | tail -n +336 && echo "---" && head -n 285 src/renderer/src/pages/player/components/SubtitleOverlay.tsx | tail -n +275 && echo "---" && head -n 96 src/renderer/src/pages/player/components/SubtitleOverlay.tsx | tail -n +90Length of output: 867
🏁 Script executed:
#!/bin/bash
# Final definitive check
python3 << 'PYEOF'
try:
with open('src/renderer/src/pages/player/components/SubtitleOverlay.tsx', 'r') as f:
lines = f.readlines()
print("=== Total lines:", len(lines))
print("\n=== Lines 90-96:")
for i in range(89, 96):
if i < len(lines):
print(f"{i+1}: {lines[i]}", end="")
print("\n\n=== Lines 275-285:")
for i in range(274, 285):
if i < len(lines):
print(f"{i+1}: {lines[i]}", end="")
print("\n\n=== Lines 338-343:")
for i in range(337, 343):
if i < len(lines):
print(f"{i+1}: {lines[i]}", end="")
# Check for key terms
full_content = ''.join(lines)
print("\n\n=== Key Checks:")
print("isFocused state:", "YES" if "isFocused" in full_content else "NO")
print("handleFocus function:", "YES" if "handleFocus" in full_content else "NO")
print("handleBlur function:", "YES" if "handleBlur" in full_content else "NO")
print("onFocus binding:", "YES" if "onFocus=" in full_content else "NO")
print("onBlur binding:", "YES" if "onBlur=" in full_content else "NO")
except Exception as e:
print(f"Error: {e}")
PYEOFLength of output: 1150
在遮罩模式下增加焦点状态处理以改进键盘可达性
验证确认：组件当前仅在 hover 时显示调整大小句柄，缺少键盘聚焦支持。虽然 OverlayContainer 已设置 tabIndex={0}，但键盘用户 Tab 聚焦时无法看到或操作尺寸柄。
建议的修复是正确且必要的。需要按以下方式实施：
- 第 90-96 行 - 在本地状态中添加
isFocused状态：
const [toastType, setToastType] = useState<'copy' | 'mask-onboarding' | null>(null)
const hideToastTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null)
+ const [isFocused, setIsFocused] = useState(false)- 第 275-285 行 - 在事件处理器中添加焦点处理函数：
const handleMouseLeave = useCallback(() => {
if (!isDragging && !isResizing) {
setHovered(false)
}
}, [setHovered, isDragging, isResizing])
+
+ const handleFocus = useCallback(() => setIsFocused(true), [])
+ const handleBlur = useCallback(() => setIsFocused(false), [])- 第 338-343 行 - 在 OverlayContainer 上绑定焦点事件，并更新可见性条件：
<OverlayContainer
onMouseEnter={handleMouseEnter}
onMouseLeave={handleMouseLeave}
+ onFocus={handleFocus}
+ onBlur={handleBlur}
>
...
<SubtitleResizeHandle
- visible={isMaskMode && isHovered}
+ visible={isMaskMode && (isHovered || isFocused)}
onMouseDown={handleResizeMouseDown}
onDoubleClick={handleResizeDoubleClick}
/>验证通过后，使用 Tab 聚焦 Overlay 应可见调整大小句柄并支持交互操作。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {/* 只在遮罩模式下且悬停时显示调整大小句柄 */} | |
| <SubtitleResizeHandle | |
| visible={isMaskMode && isHovered} | |
| onMouseDown={handleResizeMouseDown} | |
| onDoubleClick={handleResizeDoubleClick} | |
| /> | |
| const [toastType, setToastType] = useState<'copy' | 'mask-onboarding' | null>(null) | |
| const hideToastTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null) | |
| const [isFocused, setIsFocused] = useState(false) |
Suggested change
| {/* 只在遮罩模式下且悬停时显示调整大小句柄 */} | |
| <SubtitleResizeHandle | |
| visible={isMaskMode && isHovered} | |
| onMouseDown={handleResizeMouseDown} | |
| onDoubleClick={handleResizeDoubleClick} | |
| /> | |
| const handleMouseLeave = useCallback(() => { | |
| if (!isDragging && !isResizing) { | |
| setHovered(false) | |
| } | |
| }, [setHovered, isDragging, isResizing]) | |
| const handleFocus = useCallback(() => setIsFocused(true), []) | |
| const handleBlur = useCallback(() => setIsFocused(false), []) |
Suggested change
| {/* 只在遮罩模式下且悬停时显示调整大小句柄 */} | |
| <SubtitleResizeHandle | |
| visible={isMaskMode && isHovered} | |
| onMouseDown={handleResizeMouseDown} | |
| onDoubleClick={handleResizeDoubleClick} | |
| /> | |
| <OverlayContainer | |
| onMouseEnter={handleMouseEnter} | |
| onMouseLeave={handleMouseLeave} | |
| onFocus={handleFocus} | |
| onBlur={handleBlur} | |
| > | |
| {/* 只在遮罩模式下且悬停时显示调整大小句柄 */} | |
| <SubtitleResizeHandle | |
| visible={isMaskMode && (isHovered || isFocused)} | |
| onMouseDown={handleResizeMouseDown} | |
| onDoubleClick={handleResizeDoubleClick} | |
| /> |
🤖 Prompt for AI Agents
In src/renderer/src/pages/player/components/SubtitleOverlay.tsx around lines
338-343 (and related spots at ~90-96 and ~275-285), the resize handle is only
shown on hover so keyboard users who Tab into the OverlayContainer can't see or
use it; add a local boolean state isFocused (initialize near lines 90-96), add
onFocus and onBlur handlers (near 275-285) that set isFocused true/false, attach
those handlers to the OverlayContainer (ensure tabIndex remains set) and update
the SubtitleResizeHandle visible prop to be visible={isMaskMode && (isHovered ||
isFocused)} so the handle appears and is operable when the overlay receives
keyboard focus.
Comment on lines
+121
to
+128
| // === 字幕变化时清除选择的文本 === | ||
| useEffect(() => { | ||
| if (selectedText) { | ||
| setSelectedText('') | ||
| logger.debug('字幕变化时清除选择的文本') | ||
| } | ||
| }, [currentSubtitleData?.index, selectedText, setSelectedText]) | ||
|
|
Contributor
There was a problem hiding this comment.
副作用依赖导致“每次选中文本都会被立刻清空”
useEffect 依赖包含 selectedText，当用户划词设置了选中值时，副作用立即触发并清空，造成选中无法保留。应仅在“字幕索引变化”时清空。
应用如下修复使用上一索引对比，既满足 ESLint 依赖，又不误清空：
+ const prevIndexRef = React.useRef<number | null>(null)
// === 字幕变化时清除选择的文本 ===
useEffect(() => {
- if (selectedText) {
- setSelectedText('')
- logger.debug('字幕变化时清除选择的文本')
- }
- }, [currentSubtitleData?.index, selectedText, setSelectedText])
+ const currIndex = currentSubtitleData?.index ?? null
+ const indexChanged = prevIndexRef.current !== null && currIndex !== prevIndexRef.current
+ if (indexChanged && selectedText) {
+ setSelectedText('')
+ logger.debug('字幕变化时清除选择的文本', { indexFrom: prevIndexRef.current, indexTo: currIndex })
+ }
+ prevIndexRef.current = currIndex
+ }, [currentSubtitleData?.index, selectedText, setSelectedText])Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/renderer/src/pages/player/hooks/useSubtitleOverlay.ts around lines
121-128, the current useEffect clears selectedText immediately because
selectedText is included in the dependency array; change the effect to only run
when the subtitle index changes by tracking the previous index (use a useRef
like prevIndexRef), compare prevIndexRef.current with currentSubtitleData?.index
and only call setSelectedText('') and logger.debug(...) when they differ, then
update prevIndexRef.current = currentSubtitleData?.index; keep setSelectedText
and any stable refs in the deps to satisfy ESLint, but remove selectedText from
the dependency array so user selections are not cleared immediately.
Add updateLatestSize function to useSubtitleDrag hook to ensure latestSizeRef is updated when overlay size changes. This fixes incorrect bottom boundary detection during drag operations after overlay height changes. - Add updateLatestSize callback in useSubtitleDrag hook - Use updateLatestSizeFromDrag in SubtitleOverlay component - Ensure drag boundary calculations use current overlay dimensions Fixes issue where overlay bottom boundary detection was incorrect after height changes, as drag calculations were using stale size references.
- remove SubtitleDictionaryLookup.test.tsx - this test file is no longer needed
github-actions Bot
pushed a commit
that referenced
this pull request
Oct 21, 2025
# [1.4.0](v1.3.0...v1.4.0) (2025-10-21) ### Features * **player/subtitle:** comprehensive subtitle overlay improvements ([#226](#226)) ([7b749a7](7b749a7))
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
发版说明
新功能
改进