fix(lyrics): preserve share action icons on short screens by cwuom · Pull Request #620 · cwuom/NeriPlayer

cwuom · 2026-10-30T12:16:50Z

描述 / Description

Reserve space for lyric share actions before measuring the scrollable lyrics list, preventing action icons from collapsing on short screens. Preserve the existing 430dp list height limit and content-based sizing.

Add Compose regression coverage for short portrait, landscape, and tall portrait layouts, including icon dimensions, scrolling, selection, copying, the 430dp height cap, and content-based height for short lyric lists.

类型 / Type

Bug 修复 / Bug Fix
新功能 / New Feature
文档更新 / Documentation Update
其他（请描述）/ Other (please describe):

修复或解决的问题 / Issues Fixed or Closed by This PR

Fixes #597

清单 / Checklist

我已阅读并遵循贡献指南 / I have read and followed the contribution guidelines
我已在本地测试这些更改 / I have tested these changes locally
我已更新相关文档或注释（如适用） / I have updated relevant documentation or comments (if applicable)
我确认本次更改没有破坏任何原有逻辑 / I confirm this change does not break existing behavior
我确认本次更改保持向下兼容（如适用） / I confirm this change remains backward compatible (if applicable)
我已运行必要的构建、测试或静态检查 / I have run the required build, tests, or static checks
我确认没有提交密钥、Token 或其他敏感信息 / I confirm no secrets, tokens, or other sensitive information were committed
涉及 UI 变更时，我已补充截图或录屏（如适用） / I have added screenshots or recordings for UI changes (if applicable)

其他信息 / Additional Information

179 lyrics JVM/Compose tests passed. Temporary fill = true and 600.dp maximum-height mutations failed the short-list and tall-list height assertions, respectively; production code was restored before the passing test run. lintDebug --offline and assembleDebug passed; lint reported 50 warnings outside the changed files.

The arm64 DEBUG APK was installed over the existing installation on OnePlus PLK110. The installed APK hash matched the build, and the original installation time and data-directory identities were preserved. On-device UI interaction has not been tested.

Documentation changes are not needed for this layout correction. No new screenshots or recordings are attached.

歌词分享面板先为操作按钮保留布局空间，再测量歌词列表。短屏幕上，复制、分享歌曲和分享歌词卡片图标保持 24dp × 24dp。歌词较多时，列表可滚动，最大高度仍为 430dp；歌词较少时，列表按内容高度布局。

新增 Compose 测试覆盖短竖屏、短横屏和高竖屏，验证图标尺寸、列表滚动、歌词选择和复制内容。另新增高竖屏短歌词测试，验证列表边界与首末歌词对齐。

chatgpt-codex-connector · 2026-10-30T12:17:02Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-30T13:01:35.846145Z	`1d274a3`	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

coderabbitai · 2026-10-30T12:23:35Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Last reviewed commit: 1d274a3
Latest commit, not reviewed: 720956e

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

Walkthrough

歌词列表新增 weight(1f, fill = false) 布局约束，原有 430.dp 高度上限保持不变。新增测试覆盖短竖屏、短横屏和高竖屏配置，检查操作图标尺寸、歌词滚动选择和复制结果。

Priority: Low

Severity of issue fixed: Low

Merge Risk: Low · previous review 1d274

The layout currently works, but add the focused height assertion to protect the preserved list-size behavior from regression.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		标题准确概括了短屏幕上保留歌词分享操作图标这一主要变更。
Linked Issues check		[`#597`] 要求歌词复制页图标正常缩放。整体 PR 相对 merge base 的差异仅在 `LyricShareSheet.kt` 中为歌词 `LazyColumn` 增加 `.weight(1f, fill = false)`，并保留 `.heightIn(max = 430.dp)`。该 `LazyColumn` 是 `ModalBottomSheet` 内 `Column` 的直接子项，…
Out of Scope Changes check		整体 PR 相对 merge base 的变更包括：`LyricShareSheet.kt` 中一行布局修饰符修改，以及 `LyricShareSheetComposeTest.kt` 中的回归测试和所需的 `dp` 导入。布局修改直接对应 [`#597`] 的图标缩放问题。测试覆盖图标尺寸、滚动、选择、复制和短列表高度，均属于该修复的验证范围。未发现与 [`#597`] 无关的功能或文件变更。PR 描述…

Full details: Linked Issues check

Explanation

[#597] 要求歌词复制页图标正常缩放。整体 PR 相对 merge base 的差异仅在 LyricShareSheet.kt 中为歌词 LazyColumn 增加 .weight(1f, fill = false)，并保留 .heightIn(max = 430.dp)。该 LazyColumn 是 ModalBottomSheet 内 Column 的直接子项，因此 weight 可用。操作行（LyricShareActions）为非权重子项，会先测量；歌词列表随后只使用剩余高度，上限仍为 430dp。这为操作图标预留了空间，直接针对问题描述中的图标压缩。LyricShareSheetComposeTest.kt 新增测试覆盖短竖屏（w360dp-h540dp）、短横屏（w640dp-h360dp）和高竖屏（w411dp-h900dp）。测试断言三个操作图标的宽高均为 24.dp，30 行歌词可滚动、选择和复制，并验证短歌词列表高度与内容一致。本次未运行测试，结论基于源码与测试内容；作者报告 179 个 JVM/Compose 测试通过。

Full details: Out of Scope Changes check

Explanation

整体 PR 相对 merge base 的变更包括：LyricShareSheet.kt 中一行布局修饰符修改，以及 LyricShareSheetComposeTest.kt 中的回归测试和所需的 dp 导入。布局修改直接对应 [#597] 的图标缩放问题。测试覆盖图标尺寸、滚动、选择、复制和短列表高度，均属于该修复的验证范围。未发现与 [#597] 无关的功能或文件变更。PR 描述中的 APK 安装与设备验证属于非编码内容，不参与代码范围判定。

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Follow-up Comments

为短歌词列表增加内容高度断言。

150-170: Trivial · Functional Correctness · Quick win

为短歌词列表增加内容高度断言。

三个新增布局测试都使用 30 行歌词。它们只验证操作图标尺寸、滚动、选择和复制。若将 Modifier.weight(1f, fill = false) 改为 fill = true，这些断言仍可能通过，因此无法检测短歌词列表在有剩余空间时被扩展的回归。

请增加少量歌词的 fixture，并断言 LazyColumn 的实际高度保持为内容高度。现有其他 LyricShareSheet 测试也没有该断言。

Open the source location

Review info

Run configuration

Source mode: Auto
Repository: cwuom/NeriPlayer
Branch: fix/issue-597-lyric-actions
File: .coderabbit.yaml
Commit: c477139
Layers: Repository YAML
Review profile: QUIET
Plan: Advanced
Run ID: 8dfdfe5f-c3f2-4fe5-b742-6091b9e31077

Commits

Reviewing files that changed from the base of the PR and between 44d00ec and c477139.

Files selected for processing: 2

app/src/main/java/moe/ouom/neriplayer/ui/component/lyrics/LyricShareSheet.kt
app/src/test/java/moe/ouom/neriplayer/ui/component/lyrics/LyricShareSheetComposeTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

coderabbitai

Follow-up Comments

在高屏长列表用例中断言列表高度。

152-172: Trivial · Functional Correctness · Quick win

在高屏长列表用例中断言列表高度。

当前测试只验证滚动、选择、图标尺寸和复制内容。即使移除或增大 heightIn(max = 430.dp)，30 行歌词仍会提供 hasScrollToIndexAction()，这些断言也不会读取列表高度。在 w411dp-h900dp 配置中，列表可以超过 430.dp，导致分享面板布局回归。

请让高屏长列表用例对滚动节点直接断言 430.dp 高度。短屏用例不应要求精确的 430.dp 高度。

Suggested fix

 import androidx.compose.ui.unit.dp
+import androidx.compose.ui.unit.Dp

-    private fun assertActionsRemainUsableWithScrollableLyrics() {
+    private fun assertActionsRemainUsableWithScrollableLyrics(
+        expectedListHeight: Dp? = null
+    ) {
         val manyLyrics = List(30) { index ->
             LyricEntry(
                 text = "Lyric line $index",
                 startTimeMs = index * 1_000L,
                 endTimeMs = (index + 1) * 1_000L
             )
         }
         setSheet(sheetLyrics = manyLyrics, initialLine = manyLyrics.first())

+        expectedListHeight?.let { expectedHeight ->
+            composeRule.onNode(hasScrollToIndexAction())
+                .assertHeightIsEqualTo(expectedHeight)
+        }
         assertActionIconsKeepTheirSize()

     @Test
     @Config(qualifiers = "w411dp-h900dp-mdpi")
     fun `tall portrait keeps full size action icons and scrollable lyrics`() {
-        assertActionsRemainUsableWithScrollableLyrics()
+        assertActionsRemainUsableWithScrollableLyrics(expectedListHeight = 430.dp)
     }

Open the source location

Review info

Run configuration

Source mode: Auto
Repository: cwuom/NeriPlayer
Branch: fix/issue-597-lyric-actions
File: .coderabbit.yaml
Commit: 1d274a3
Layers: Repository YAML
Review profile: QUIET
Plan: Advanced
Run ID: 59916fdb-1b40-4483-9b60-87cfd171f3dd

Commits

Reviewing files that changed from the base of the PR and between c477139 and 1d274a3.

Files selected for processing: 1

app/src/test/java/moe/ouom/neriplayer/ui/component/lyrics/LyricShareSheetComposeTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review.

fix(lyrics): preserve share action icons on short screens

c477139

coderabbitai Bot reviewed Oct 30, 2026

View reviewed changes

test(lyrics): guard compact share sheet height

1d274a3

coderabbitai Bot reviewed Oct 30, 2026

View reviewed changes

test(lyrics): verify share list height limit on tall screens

720956e

cwuom merged commit b7fad04 into master Oct 30, 2026
12 checks passed

cwuom deleted the fix/issue-597-lyric-actions branch October 30, 2026 15:41

Navigation Menu

Repository navigation

fix(lyrics): preserve share action icons on short screens - #620

fix(lyrics): preserve share action icons on short screens#620
cwuom merged 3 commits into
masterfrom
fix/issue-597-lyric-actions

cwuom commented Oct 30, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Oct 30, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 30, 2026 •

edited

Loading

Reviews paused

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

cwuom commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

描述 / Description

类型 / Type

修复或解决的问题 / Issues Fixed or Closed by This PR

清单 / Checklist

其他信息 / Additional Information

Uh oh!

chatgpt-codex-connector Bot commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

coderabbitai Bot commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Follow-up Comments

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Follow-up Comments

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cwuom commented Oct 30, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 30, 2026 •

edited

Loading

coderabbitai Bot commented Oct 30, 2026 •

edited

Loading