Skip to content

🎨 Palette: 优化配对按钮的平滑过渡状态及加载微交互 - #192

Open
01luyicheng wants to merge 1 commit into
devfrom
palette-pairing-button-animation-3895757322148625214
Open

01luyicheng wants to merge 1 commit into
devfrom
palette-pairing-button-animation-3895757322148625214

Conversation

@01luyicheng

Copy link
Copy Markdown
Owner

💡 What: 为配对界面的按钮添加了 AnimatedContent 平滑过渡状态,取代了原本生硬的条件渲染,并修正了 CircularProgressIndicator 的颜色,使用 LocalContentColor.current 替代写死的颜色。
🎯 Why: 这提供了一个更加自然优雅的用户体验交互。修复转圈动画颜色后,如果按钮处于禁用状态,转圈颜色也能跟随保持禁用对比度一致。
📸 Before/After: (之前:无过渡状态生硬切换;现在:丝滑淡入淡出的状态切换)。
♿ Accessibility: 使用 LocalContentColor.current 确保了禁用加载状态下的对比度和可访问性。


PR created automatically by Jules for task 3895757322148625214 started by @01luyicheng

Co-authored-by: 01luyicheng <172185967+01luyicheng@users.noreply.github.com>
@google-labs-jules

Copy link
Copy Markdown

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

Copilot AI review requested due to automatic review settings August 3, 2026 12:22

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@qodo-code-review

Copy link
Copy Markdown

ⓘ Your Qodo trial ends soon. Ask your workspace admin to set up billing to keep reviews running after the trial. Manage billing

@coderabbitai

coderabbitai Bot commented Aug 3, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 502b2911-592a-4a8f-859d-41a2feda7b35

📥 Commits

Reviewing files that changed from the base of the PR and between b217d54 and 98b8d39.

📒 Files selected for processing (2)
  • .Jules/palette.md
  • android/app/src/main/java/com/netproxy/gateway/ui/screens/MainScreen.kt
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (3)
android/app/src/main/java/com/netproxy/gateway/**/*.kt

📄 CodeRabbit inference engine (CLAUDE.md)

android/app/src/main/java/com/netproxy/gateway/**/*.kt: Source of truth for runtime behavior is Kotlin code under android/app/src/main/java/com/netproxy/gateway/. If documentation conflicts with code, code is authoritative and documentation must be updated.
WiFi connection path uses traditional API limited on Android 10+.
MQTT security uses TLS 1.2 and certificate pinning when MQTT_TLS_PUBLIC_KEY_PINS is configured; falls back to default CA validation when empty.
Network egress control: cannot force Socket to use WiFi or mobile data; current protect() bypasses VPN but traffic may be routed to unintended interfaces in vendor scenarios (Link Turbo, dual WiFi acceleration).
Missing native Android multi-network APIs: code does not use Network.bindSocket(), excludeRoute(), or other APIs for true network interface binding.
Multi-network handling is incomplete: code only identifies single network type and cannot properly handle multiple simultaneous networks (dual WiFi, Bluetooth PAN, OTG wired) or network switching scenarios. See docs/TECH_DEBT.md C1 and C3.
Dependency risk: Paho MQTT client maintenance is inactive. New features and bug fixes may be delayed. See docs/ISSUES.md N15.
Deprecated API usage: large amounts of deprecated API usage including EncryptedSharedPreferences, WifiConfiguration, NioEventLoopGroup. See docs/ISSUES.md N13.
Test coverage gap: core business logic (processVpnTraffic, forwardViaSocks5) lacks test coverage. See docs/ISSUES.md N8.

Files:

  • android/app/src/main/java/com/netproxy/gateway/ui/screens/MainScreen.kt
**/*.{kt,go}

📄 CodeRabbit inference engine (CLAUDE.md)

Critical defect: connection pool cleanup race condition - connection state may change between read and write locks. See docs/ISSUES.md H5.

Files:

  • android/app/src/main/java/com/netproxy/gateway/ui/screens/MainScreen.kt
android/**

📄 CodeRabbit inference engine (CLAUDE.md)

Build validation gate: make android-build must pass.

Files:

  • android/app/src/main/java/com/netproxy/gateway/ui/screens/MainScreen.kt
🔇 Additional comments (3)
android/app/src/main/java/com/netproxy/gateway/ui/screens/MainScreen.kt (2)

608-627: LGTM!


608-627: 📐 Maintainability & Code Quality

请通过 Android 构建门禁。

android/** 下的改动必须通过 make android-build。请在合并前运行该命令,确认 Compose API 导入和编译均通过。

Source: Coding guidelines

.Jules/palette.md (1)

9-12: LGTM!


📝 Walkthrough

Summary by CodeRabbit

  • 用户体验改进
    • 配对按钮在普通状态与加载状态之间加入平滑动画过渡。
    • 加载指示器颜色会自动匹配按钮当前的内容颜色及禁用状态色调。

Walkthrough

配对按钮现在使用 AnimatedContent 切换普通状态和加载状态。加载指示器使用 LocalContentColor.current,并新增相关 Jetpack Compose 实践记录。

Changes

配对按钮状态过渡

Layer / File(s) Summary
配对按钮动画与颜色
.Jules/palette.md, android/app/src/main/java/com/netproxy/gateway/ui/screens/MainScreen.kt
PairingSection 使用 AnimatedContent 切换配对文本和加载内容。加载指示器继续显示进度与配对中提示,并使用 LocalContentColor.current。实践记录说明相同实现方式。

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested labels: 🕐 10-20 Minutes

Suggested reviewers: copilot

Poem

兔耳轻摇看按钮,
配对加载缓缓流。
AnimatedContent 换新景,
指示器随颜色走。
代码清晰,萝卜丰收。

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed 标题准确概括了配对按钮的平滑状态过渡和加载微交互优化。
Description check ✅ Passed 描述明确说明了 AnimatedContent 和 LocalContentColor.current 的修改及其用户体验和可访问性目的。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Palette: Smooth pairing button loading transition and adaptive spinner tint

✨ Enhancement 📝 Documentation 🕐 10-20 Minutes

Grey Divider

AI Description

• Animate pairing button content between idle and loading states via AnimatedContent.
• Tint the loading spinner using LocalContentColor for correct disabled-state contrast.
• Document Compose guidance for smooth button loading micro-interactions in Palette notes.
Diagram

graph TD
  A["UiState.isPairingInProgress"] --> B["PairingSection"] --> C["Pair Button"] --> D["AnimatedContent"] --> E{"isPairing?"}
  E --> F["Loading content (spinner + text)"]
  E --> G["Idle content (text)"]
  F --> H["Spinner uses LocalContentColor"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Crossfade between loading/idle content
  • ➕ Slightly simpler mental model for boolean state swaps
  • ➕ Often sufficient for pure opacity transitions
  • ➖ Less flexible for future content-size/layout changes compared to AnimatedContent
  • ➖ Harder to tune transitions if you later want size/placement animations
2. Overlay spinner on top of text (no content swap)
  • ➕ Avoids layout shifts; preserves button label for context
  • ➕ Can feel more stable for accessibility/screen recording
  • ➖ Requires more layout work (Box/constraints) and careful semantics to avoid double announcements
  • ➖ Visually busier; can reduce readability if not tuned

Recommendation: Keep the current AnimatedContent approach: it cleanly scopes derived UI to the animated lambda and provides a polished transition with minimal code. Using LocalContentColor.current for the spinner is the right choice for theme/disabled-state consistency and accessibility.

Files changed (2) +24 / -10

Enhancement (1) +20 / -10
MainScreen.ktAnimate pairing button loading state and use adaptive spinner tint +20/-10

Animate pairing button loading state and use adaptive spinner tint

• Replaces abrupt conditional rendering of pairing button content with AnimatedContent for a smooth transition between idle and loading states. Updates the progress indicator color to LocalContentColor.current so it matches button content tint, including when disabled.

android/app/src/main/java/com/netproxy/gateway/ui/screens/MainScreen.kt

Documentation (1) +4 / -0
palette.mdDocument guidance for smooth button loading transitions +4/-0

Document guidance for smooth button loading transitions

• Adds a Palette learning/action entry recommending AnimatedContent for button loading transitions. Documents using LocalContentColor.current for CircularProgressIndicator to preserve correct enabled/disabled contrast.

.Jules/palette.md

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

@qodo-code-review

Copy link
Copy Markdown

Qodo Fixer

No findings are available for this PR yet. Findings appear here once Qodo has reviewed the PR.

@Luyicheng-Agent

Copy link
Copy Markdown
Collaborator

只读复核,未修改任何代码。先说结论:本 PR 的 AnimatedContent + LocalContentColor.current 改动正确(状态/生命周期/取消处理无误,禁用态色调一致性确有改善),可合并。下面主要是给维护者的"去重"参考和一个可选优化。

三个配对按钮 PR 是近重复,建议只合其一

CodeRabbit 在本 PR 的 walkthrough 里已把 #185 / #191 列为「Possibly related PRs」(指出改动相同)。我这里补一个具体的合并/关闭建议 + 三方结构对比,供维护者定夺。

#185 / #191 / #192 都是同一特性(MainScreen.kt 的 PairingSection 按钮内容包入 AnimatedContent + spinner 颜色由硬编码 onPrimary 改为 LocalContentColor.current),enabled / onClick 不变、配对流程不受影响。三者差异:

三者只应合并其一。从结构与描述完整度看 #192 最完整,建议合并 #192、关闭 #185 与 #191(我无权关闭,留给维护者决定)。合并前建议跑一次 make android-build。

一个可选优化(非阻塞)

尊重系统"减弱动态效果":Compose 的 AnimatedContent 不会自动响应系统的"移除动画"设置。可在切换前读取 Settings.Global.ANIMATOR_DURATION_SCALE(为 0 表示用户在开发者选项里关闭了动画),在该情况下用 EnterTransition.None / ExitTransition.None 替换过渡,照顾对动效敏感的用户。这是通用可访问性最佳实践,非本 PR 引入的问题,可不阻塞合并。

注:CodeRabbit 在 #185 提到的"animation 依赖 Major"应为误报——dev 已多处使用 AnimatedContent,依赖经 BOM/material3 传递可用。

以上为只读复核,最终取舍由维护者决定。

@Luyicheng-Agent

Copy link
Copy Markdown
Collaborator

Post-commit correctness review — no high-impact bug ✅ (one low-sev note)

Reviewed as part of a multi-subagent sweep of the past 24h of commits. Commit 98b8d39 touches only MainScreen.kt (+ a palette note). The change is purely cosmetic: the PairingButton's if(isPairingInProgress){spinner}else{Text} is wrapped in AnimatedContent, the spinner color moves to LocalContentColor.current, and content is wrapped in a Row. No state logic, enabled expression, or onClick is changed.

Stuck-loading analysis (the main concern): safe. On the success path the button relies on the MQTT observer to clear isPairingInProgress; I traced MqttConnectionManager.connect and Paho's connectionTimeout = 10s guarantees a terminal MQTT state (Connected/Error/Disconnected) that clears the flag. Every error path (catch / no-cellular / MQTT Error/Disconnected) resets isPairingInProgress=false, so the button re-enables — not permanently blocked. No LaunchedEffect/remember keying issue, no nullable deref, no re-entry state strand. Disabling during loading correctly prevents double-tap double-pairing.

Low-severity note (F-192-1): the spinner color (LocalContentColor.current) and the Text default color are bound to the Button's enabled = ... && !uiState.isPairingInProgress, which flips to false immediately on transition. So the fading-out "Pair" text color-pops (dims to the disabled tint) at transition start, then fades. This re-instantiates the same AnimatedContent anti-pattern class already documented as REV62/REV63 (docs/ISSUES.md): derived visual property bound to shared outer state instead of the per-target lambda parameter. Impact here is much smaller than REV62/REV63 (a ~300ms dim of a button label, no crash/functional/security impact). Per the .Jules/palette.md learning, the spinner-in-disabled-tint is intended; only the outgoing text's color-pop is unintended. Optional fix: derive the color from the isPairing target parameter inside the lambda so the outgoing row retains its original enabled tint.

Pre-existing (not introduced): observeMqttState's Connecting branch sets isPairingInProgress=true, so the loading flag also fires during background reconnects — the button can flicker idle↔loading on reconnect cycles. PR #192 doesn't cause this (its AnimatedContent actually softens it to a crossfade).

LGTM from a correctness standpoint; the color-pop is optional polish.

@Luyicheng-Agent

Copy link
Copy Markdown
Collaborator

提交后交叉审查(多 subagent)

与 #195 同类的配对按钮平滑过渡 + 对比度修复。审查结论一致:AnimatedContent/LocalContentColor.current 用法正确,无新缺陷。建议与 #195/#191 择一保留,避免多个重复分支长期并存。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants