Skip to content

refactor(core): 将点击相关事件和样式由歌词行上移至歌词组#538

Merged
apoint123 merged 3 commits into
mainfrom
refactor/elevate-events-and-styles-to-group
May 22, 2026
Merged

refactor(core): 将点击相关事件和样式由歌词行上移至歌词组#538
apoint123 merged 3 commits into
mainfrom
refactor/elevate-events-and-styles-to-group

Conversation

@apoint123

@apoint123 apoint123 commented May 18, 2026

Copy link
Copy Markdown
Member

摘要

将原本分散在单行歌词上的鼠标事件监听、hover/active 遮罩效果以及内外边距控制上移至歌词组,以获得更一致的视觉和点击效果

改动点

  • 移除了在单个 LyricLineEl 上遍历绑定鼠标事件的逻辑,改为在 DomLyricPlayer 根节点挂载唯一的 clickcontextmenu 监听器,用 lyricGroupElementMap 实现 O(1) 路由
  • LyricLineMouseEvent 的基础上新增 bgLine 属性,下游消费者可以获取到点击的歌词组的背景人声行。
  • 移除了 lyric-line.ts 中不再需要的 RawLyricLineMouseEventlistenersMap 及其增删方法
  • 移除了 .lyricLine.lyricBgLine 各自的 paddingborder-radius,由外层组容器 .lyricLineWrapper 统一处理内外边距
  • :hover:active 触发的背景高亮交互上移至歌词组,可以同时高亮包含主歌词和背景人声歌词的歌词组,而非之前单独高亮主歌词行和背景人声歌词行
  • 略微减小了主歌词和背景人声歌词之间的间隙
  • 移除了未知作用,似乎是遗留代码的 .lyricLine 上的 .dirty 样式

验证

works on my machine

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

该 PR 将歌词行的点击/右键事件监听与 hover/active 高亮、内外边距等样式从“单行歌词”上移到“歌词组容器”,以统一交互与视觉表现,并通过元素映射实现事件 O(1) 路由。

Changes:

  • 将 click/contextmenu 事件监听集中到 DomLyricPlayer 根节点,并基于 .lyricLineWrapper + lyricGroupElementMap 路由到对应歌词组
  • 扩展 LyricLineMouseEvent,新增 bgLine 字段以暴露背景人声行(若存在)
  • 调整歌词组/背景行相关样式:padding/radius/hover/active 统一上移到组容器,并更新背景行容器定位逻辑

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
packages/core/src/styles/lyric-player.module.css 将 hover/active 与内外边距上移到歌词组容器,并调整背景人声容器布局/定位
packages/core/src/lyric-player/dom/lyric-line.ts 移除逐行鼠标事件封装与监听器管理逻辑
packages/core/src/lyric-player/dom/index.ts 改为根节点事件委托,事件对象新增 bgLine 并通过组容器映射路由
.nx/version-plans/version-plan-1779103936447.md 为 core-bundle 增加 patch 版本计划

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/core/src/styles/lyric-player.module.css
Comment thread packages/core/src/lyric-player/dom/index.ts Outdated
Comment thread packages/core/src/lyric-player/dom/index.ts
@apoint123 apoint123 force-pushed the refactor/elevate-events-and-styles-to-group branch from b1e4fe7 to 9b632b7 Compare May 18, 2026 12:08
@apoint123 apoint123 requested a review from Copilot May 18, 2026 12:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread packages/core/src/lyric-player/dom/index.ts Outdated
@apoint123 apoint123 force-pushed the refactor/elevate-events-and-styles-to-group branch from 9b632b7 to 026bcb3 Compare May 18, 2026 12:21
@apoint123 apoint123 force-pushed the refactor/elevate-events-and-styles-to-group branch from 026bcb3 to 58aa4b6 Compare May 18, 2026 12:51
@apoint123 apoint123 requested a review from Linho1219 May 18, 2026 12:52
Comment thread packages/core/src/lyric-player/dom/index.ts
Comment thread packages/core/src/styles/lyric-player.module.css
@apoint123

This comment has been minimized.

@apoint123 apoint123 marked this pull request as draft May 19, 2026 10:36
@apoint123 apoint123 marked this pull request as ready for review May 20, 2026 04:57
@apoint123 apoint123 merged commit 3879909 into main May 22, 2026
4 checks passed
@apoint123 apoint123 deleted the refactor/elevate-events-and-styles-to-group branch May 22, 2026 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants