fix: Remove mobile device detection #719
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough移除 StepHandler.tsx 中基于移动端的渲染拦截逻辑，使组件在所有设备上无条件渲染；同时删除了依赖该行为的移动端相关测试文件 tests/mobile.test.tsx。其余处理步骤、鼠标按下循环与清理逻辑未改动。 Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as StepHandler
participant N as InputNumber
rect rgba(230, 245, 255, 0.6)
note right of S: 新行为：不区分设备，始终渲染
U->>S: 查看组件（移动/桌面）
S->>U: 渲染步进按钮 +/- 与事件绑定
end
U->>S: 按下/长按 +/-
activate S
S->>N: 触发步进回调（递增/递减）
N-->>S: 更新值
S-->>U: 反映最新值
deactivate S
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @Wxh16144, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
此拉取请求旨在改进 InputNumber 组件中增减控件的显示逻辑，特别是针对移动设备。它为这些元素的可见性提供了更灵活的控制，允许开发者在需要时显式地在移动设备上显示它们，而不是无条件隐藏。
Highlights
- 控件显示逻辑优化: 核心改动是关于增减控件（
StepHandler）的显示逻辑，尤其是在移动设备上。 - 移动端控件可见性: 之前，
StepHandler在移动设备上是无条件隐藏的。现在，StepHandler的可见性由mergedControls变量控制，该变量默认值为!isMobile，但可以通过controls属性显式设置。这意味着controls=true将强制在移动设备上显示控件。 - 代码结构调整:
useMobilehook 从StepHandler.tsx移动到InputNumber.tsx，以集中移动检测逻辑。 - 测试用例更新:
mobile.test.tsx中新增了一个测试用例，用于验证当controls=true时，步进控件在移动设备上能正确显示。
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
本次 PR 旨在优化 InputNumber 组件在移动设备上增减控制器（controls）的显示逻辑。改动后，控制器在移动设备上默认隐藏，但在桌面设备上默认显示，这改善了移动端的默认用户体验。同时，通过 controls 属性，开发者仍然可以显式控制控制器的显示或隐藏，为开发者提供了更大的灵活性。
主要改动分析：
- 逻辑上移：将控制器的显示逻辑从
StepHandler组件上移至InputNumber组件。这使得StepHandler成为一个纯粹的展示组件，遵循了容器组件与展示组件分离的良好实践，是一次很好的重构。 - 动态默认值：通过引入
useMobilehook 来检测移动设备，并结合??操作符实现了controls属性的动态默认值。新的默认行为是：在移动端为false（隐藏），非移动端为true（显示）。这个逻辑const mergedControls = controls ?? !isMobile;简洁且正确地实现了需求。 - 开发者控制：新的实现允许开发者通过传递
controlsprop (e.g.,controls={true}) 来覆盖默认行为，在移动设备上也能渲染控制器 DOM。这解决了之前在移动端无法显示控制器的问题，并给予了开发者使用 CSS (如@media(hover:none)) 进行进一步样式控制的能力。 - 测试覆盖：添加了新的单元测试
can show steps when set controls to true，有效地验证了新功能。现有的测试也依然通过，保证了原有功能和 SSR 的稳定性。
整体来看，这些改动设计良好，代码清晰，并且有相应的测试覆盖。这是一个高质量的改进。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/InputNumber.tsx (3)
137-138: controls 默认语义发生变更（移动端默认隐藏）—需要在文档/变更日志中明确说明之前默认展示步进器，如今在移动端 controls 未显式设置时会隐藏。请在 README/CHANGELOG 标注该行为变化及迁移指引。需要的话我可以起草一段文档。
180-182: mergedControls 逻辑清晰，但可能引发 SSR→移动端水合不一致SSR 时常量 isMobile 初值多为 false，会服务端输出步进器；移动端客户端水合后又移除，存在结构不一致与潜在水合警告/首帧抖动。可选缓解：
- 保留 DOM，用 CSS 隐藏（避免删节点），让水合结构一致；或
- 在文档中提示该差异，并给出“如需一致，请 controls 显式设为 true/false”的建议。
614-623: 条件渲染与“可通过 @media(hover:none) 强制显示”描述不一致当前在移动端默认不渲染 StepHandler，CSS 无法“强制显示”不存在的 DOM。若要支持 CSS 覆盖，建议默认渲染但用样式隐藏，或新增类似 alwaysRenderControls/renderControlsDom 的开关以“渲染但隐藏”。请确认 PR 目标与实现保持一致。
tests/mobile.test.tsx (1)
21-24: 新增用例覆盖移动端 controls=true 的路径，👍建议补充：
- 桌面环境下 controls={false} 时不显示步进器；
- 移动端 controls 未设置时不显示（已有），设置为 false 也不显示（显式覆盖默认）。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/InputNumber.tsx(4 hunks)src/StepHandler.tsx(0 hunks)tests/mobile.test.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- src/StepHandler.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-09-29T06:18:11.993Z
Learnt from: bombillazo
PR: react-component/input-number#644
File: src/InputNumber.tsx:173-174
Timestamp: 2024-09-29T06:18:11.993Z
Learning: In `src/InputNumber.tsx`, within the `InternalInputNumber` component, the state variables `prevValueRef` and `inputValueRef` are typed as `string | number` to maintain consistency with existing code.
Applied to files:
src/InputNumber.tsxtests/mobile.test.tsx
🔇 Additional comments (4)
src/InputNumber.tsx (1)
24-24: 确认 useMobile 的 SSR 行为与副作用请确保该 hook 在 SSR 环境返回稳定初值且不触发 useLayoutEffect 警告；并确认它内部依赖的 isMobile 可被测试中的 mock 正确影响。若初值恒为 false，会与客户端首次渲染产生结构差异（见下文水合风险）。
tests/mobile.test.tsx (3)
2-3: 导入调整合理与新的导出位置保持一致，无功能性问题。
12-12: 纯格式化变动无须进一步修改。
19-20: SSR 断言可能掩盖水合差异此断言要求 SSR 输出包含 handler，但在移动端客户端水合后会移除。建议新增一条专门验证“SSR 标记存在但移动端水合后被移除”的用例，或在文档中注明这一差异为预期。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #719 +/- ##
==========================================
- Coverage 96.01% 95.97% -0.05%
==========================================
Files 7 7
Lines 301 298 -3
Branches 79 78 -1
==========================================
- Hits 289 286 -3
Misses 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
这变化有点break,不需要放 feature 吗？ |
src/InputNumber.tsx
Outdated
|
|
||
| // Mobile user experience is not ideal, so we choose to hide the step controls by default on mobile. | ||
| // However, developers can still display them by setting `controls=true`. | ||
| const isMobile = useMobile(); |
There was a problem hiding this comment.
嗯， antd v6 不需要做任何操作了
Wxh16144
changed the title
对，上一个解决方案是想 v5 动态默认值 （pc 默认显示，移动端默认隐藏），开发者仍然可以通过 controls 控制总是显示。 考虑到 v5和 v6同时维护的成本最终只考虑在 v6中进行放开。 v5 和 v4 就不发 patch 了 |
4 participants
bg: ant-design/ant-design#54827
fix #355
移除移动端条件判断。在 antd v6 生效。
原来 v4.13.0 至 v5 都不再进行修复改进了
Outdated description
移动端不应该直接去掉 dom，还是允许开发者通过
controls=true显示dom。并且开发者还可以通过@media(hover:none)始终显示。Summary by CodeRabbit
新功能
测试