Skip to content

fix: tab switch loading state and full-view loading overlay#2

Closed
obkim-hello wants to merge 1 commit intoandroidZzT:mainfrom
obkim-hello:obk/issue/1
Closed

fix: tab switch loading state and full-view loading overlay#2
obkim-hello wants to merge 1 commit intoandroidZzT:mainfrom
obkim-hello:obk/issue/1

Conversation

@obkim-hello
Copy link
Copy Markdown

  • Tab switcher now calls selectSource() to trigger refresh + loading state
  • selectSource() syncs activeTab so tab highlight stays in sync
  • Add full-view loading overlay with TimelineView-based spinning ring
  • Add L10n.loading for bilingual "Loading" / "加载中" text

- Tab switcher now calls selectSource() to trigger refresh + loading state
- selectSource() syncs activeTab so tab highlight stays in sync
- Add full-view loading overlay with TimelineView-based spinning ring
- Add L10n.loading for bilingual "Loading" / "加载中" text

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@androidZzT
Copy link
Copy Markdown
Owner

🤖 Code Review

📋 概要

修复 tab 切换时的加载状态,新增全局 loading 遮罩,tab 点击改为走 selectSource() 统一逻辑。

✅ 优点

  1. loading 动画用 TimelineView 实现旋转效果,流畅
  2. tab 点击改为调 selectSource(),比直接改 activeTab 更正确

⚠️ 问题

  • 🟡 L10n.loading 重复:当前 main 分支已有此字段(L10n.swift:137),PR 基于旧版在 line 62 又加了一个,合入会冲突
  • 🟡 loading 遮罩重复:main 分支已有 loading overlay(DashboardView 122-140 行),会出现两个遮罩
  • 🟡 冲突量大:PR 基于旧版代码,main 分支已新增 Gemini 支持、速率限制面板、新手指引框架(GuideOverlay)等大量改动,解冲突成本高

🏁 结论

❌ 建议关闭 — 改动意图好,但基于旧版本,和当前代码冲突严重。核心改进(tab 切换走 selectSource)已在后续版本中实现。感谢贡献!如果有新的想法欢迎基于最新 main 重新提 PR。

Copy link
Copy Markdown
Owner

@androidZzT androidZzT left a comment

Choose a reason for hiding this comment

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

This PR needs to be rebased onto the latest main branch before it can be reviewed. Several PRs have been merged since this was opened (including significant refactoring of StatsViewModel in PR #6), so there will likely be conflicts that need resolving. Please rebase and force-push.

Copy link
Copy Markdown
Owner

@androidZzT androidZzT left a comment

Choose a reason for hiding this comment

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

感谢提交这个 PR!Loading overlay 的方向是对的,但发现几个需要修复的问题:

需要修改的问题

1. Loading Overlay 重复(HIGH)

新增的全屏 loading overlay 与现有的 overlay(约 line 143-160)同时存在。当 isLoading == true 时两个 overlay 会叠加显示。

建议:移除旧的 overlay,只保留新的 TimelineView 版本。

2. 动画旋转速度过快(MEDIUM)

.rotationEffect(.degrees(timeline.date.timeIntervalSince1970 * 200))

timeIntervalSince1970 是一个很大的数字(~1.7 billion),乘以 200 后旋转角度极大,虽然视觉上取模后可能看起来还行,但建议用 truncatingRemainder 控制旋转周期:

.rotationEffect(.degrees(timeline.date.timeIntervalSince1970.truncatingRemainder(dividingBy: 3.6) * 100))

3. Tab 切换与数据刷新的时序(LOW)

selectSource()activeTab 同步更新,但 refresh() 是异步的。在 refresh 完成前,tab 高亮已切换但数据还是旧的。建议确认这个中间状态的 UX 是否符合预期。


其他方面(L10n、代码风格)都没问题。修复 overlay 重复问题后可以合并。

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.

2 participants