Skip to content

Conversation

Mystery00
Copy link
Contributor

@Mystery00 Mystery00 commented Sep 27, 2025

可能是因为Pixel 10 Pro上前置摄像头很高,所以设备的状态栏高度也很高,理论上不应该有什么兼容问题,但是在MainActivity中有一个自动隐藏的逻辑:
https://github.com/v2er-app/Android/blob/main/app/src/main/java/me/ghui/v2er/module/home/MainActivity.java#L244-L256
根据日志打印的情况,verticalOffset的值一直为0,但是toolbarHeight的值为125,statusBarHeight为151,判定走了隐藏ToolBar的逻辑,最终导致设备上不显示ToolBar,但是会显示ToolBar的区域,造成界面出现显示错误。
mNavigationView 组件也有相同的情况,但是那是因为没有设置Padding/MarginTop的原因。

相关截图:
图片
Screenshot_20250928-000955

相关打印代码:

Log.i("TAG", "onOffsetChanged: " + verticalOffset + ", " + toolbarHeight + ", " + statusBarHeight);
图片

- 为抽屉导航视图(NavigationView)添加状态栏内边距,以防止内容与状态栏重叠。
- 修正了在`AppBarLayout`滚动时工具栏(Toolbar)可见性的判断逻辑,确保其在向上滚动时能正确隐藏。
@Mystery00
Copy link
Contributor Author

@graycreate

@graycreate
Copy link
Member

@claude help review

Copy link

claude bot commented Sep 29, 2025

Claude finished @graycreate's task —— View job


PR Review: ToolBar Display Fix for Pixel 10 Pro

Todo List:

  • Read the modified MainActivity.java file to understand the fix
  • Analyze the toolbar auto-hide logic and root cause
  • Evaluate the fix effectiveness and potential issues
  • Provide comprehensive review feedback

Analysis

Root Cause Identified
The issue occurs in the onOffsetChanged method at lines 244-256. The problem is that on Pixel 10 Pro:

  • verticalOffset is always 0 (AppBar not scrolling)
  • toolbarHeight is 125px
  • statusBarHeight is 151px (unusually high due to front camera)
  • The condition Math.abs(verticalOffset) >= toolbarHeight - statusBarHeight evaluates to 0 >= 125 - 151 = -26, which is true

This causes the toolbar to be hidden immediately when it should remain visible.

Fix Analysis
The PR changes line 251 from:

if (Math.abs(verticalOffset) >= toolbarHeight - statusBarHeight) {

to:

if (Math.abs(verticalOffset) > toolbarHeight) {

Review Feedback

✅ Positive Aspects:

  1. Correct Root Cause: The fix addresses the mathematical issue where toolbarHeight - statusBarHeight becomes negative on devices with large status bars
  2. Logical Approach: Using Math.abs(verticalOffset) > toolbarHeight makes sense - hide toolbar only when it has scrolled beyond its own height
  3. Minimal Change: Low-risk fix that doesn't affect the overall scrolling behavior architecture

⚠️ Concerns & Questions:

  1. Loss of Status Bar Awareness: The original logic tried to account for status bar height, presumably to avoid toolbar content appearing under the status bar. Does removing this consideration cause any visual issues?

  2. Edge Case Testing: How does this change affect:

    • Normal devices with standard status bar heights?
    • Devices with notches/cutouts but normal status bar heights?
    • Different screen densities and orientations?
  3. State Management: The onStateChanged method (lines 258-269) still handles EXPANDED/COLLAPSED states. Does this change create any conflicts between the offset-based hiding and state-based visibility?

Recommendations:

  1. Consider a Hybrid Approach: Instead of completely removing status bar consideration, consider:

    // Only hide when scrolled significantly and avoid negative thresholds
    int hideThreshold = Math.max(toolbarHeight - statusBarHeight, toolbarHeight / 2);
    if (Math.abs(verticalOffset) >= hideThreshold) {
  2. Add Bounds Checking: Ensure the calculation handles edge cases:

    if (statusBarHeight <= toolbarHeight && Math.abs(verticalOffset) >= toolbarHeight - statusBarHeight) {
        // Original logic for normal devices
    } else if (Math.abs(verticalOffset) > toolbarHeight) {
        // Fallback for devices with unusual status bar heights
    }
  3. Testing: Verify the fix on multiple device types, especially:

    • Standard Android devices
    • Devices with different notch/cutout configurations
    • Various API levels and screen densities

Overall Assessment: LGTM with Minor Concerns

The fix correctly addresses the immediate issue and uses sound logic. While there are theoretical concerns about removing status bar awareness, the practical impact appears minimal since the toolbar positioning is handled by the AppBarLayout system.

@graycreate graycreate added the bug Something isn't working label Sep 29, 2025
@graycreate
Copy link
Member

graycreate commented Sep 29, 2025

@Mystery00 感谢你的贡献!我测试了这个PR,发现了一个问题需要修正。

问题分析

你的修改中添加了 verticalOffset > 0 的条件,但实际上 verticalOffset 的值:

  • 当 AppBar 完全展开时为 0
  • 当向上滚动(AppBar 收缩)时为负数
  • 在正常操作中永远不会是正数

因此 verticalOffset > 0 这个条件永远不会为真,这导致了 PR #130 中修复的滚动问题出现了回归(regression)。

建议修改

将条件从 verticalOffset > 0 改为 verticalOffset < 0

// 只在实际滚动时隐藏toolbar(verticalOffset < 0)
if (verticalOffset < 0 && Math.abs(verticalOffset) >= toolbarHeight - statusBarHeight) {
    mToolbar.setVisibility(View.INVISIBLE);
} else {
    mToolbar.setVisibility(View.VISIBLE);
}

测试结果

我在 Pixel 8 上测试了这个修改:

需要你确认

由于我没有 Pixel 10 Pro 设备,请你在你的设备上验证这个修改是否也解决了原始问题。理论上这个修改应该可以同时解决两个问题:

  1. Pixel 10 Pro 上 toolbar 消失的问题(当 verticalOffset = 0 时保持可见)
  2. 滚动时 toolbar 与状态栏重叠的问题(当滚动到一定位置时隐藏)

请更新你的代码并测试,谢谢!


🤖 此评论由 Claude Code 生成

@Mystery00
Copy link
Contributor Author

修改为 verticalOffset < 0 能解决原始问题,已经自测了

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants