-
-
Notifications
You must be signed in to change notification settings - Fork 229
feat: Support mobile config #528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Walkthrough此次变更主要增强了系统对移动端的支持。更新内容涵盖样式、文档、示例、组件、hooks 及测试用例。主要改动包括在移动端动画、过渡效果与事件处理方面的优化,例如新增移动专用样式、引入 MobileConfig 配置、调整触摸事件逻辑和更新测试断言。 Changes
Sequence Diagram(s)sequenceDiagram
participant 用户 as User
participant 触发器 as Trigger
participant 弹出层 as Popup
用户->>触发器: 发起 onTouchStart 事件
触发器->>触发器: 设置 touchedRef 为 true
用户->>触发器: 触发 onMouseEnter/onPointerEnter 事件
触发器->>触发器: 检查 mobile 配置和触摸状态
触发器->>弹出层: 条件满足后显示 Popup
弹出层-->>用户: 展示 Popup 内容
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
.eslintrc.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.js » /node_modules/.pnpm/@umijs[email protected][email protected]/node_modules/@umijs/fabric/dist/eslint.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. docs/examples/mobile.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.js » /node_modules/.pnpm/@umijs[email protected][email protected]/node_modules/@umijs/fabric/dist/eslint.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. src/index.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.js » /node_modules/.pnpm/@umijs[email protected][email protected]/node_modules/@umijs/fabric/dist/eslint.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
docs/examples/mobile.tsx (1)
55-59
: 添加移动端识别逻辑的说明缺少对如何检测移动端环境的解释。根据PR描述,这需要包装组件来决定是否使用移动端配置。建议添加注释说明移动端识别的推荐实现方式。
// mobile={{ // mask: true, // motion: { motionName: 'raise' }, // maskMotion: { motionName: 'fade' }, // }} + // 注意:此组件不直接提供移动端检测API + // 推荐在外层组件中根据环境判断后提供mobile配置 + // 例如:mobile={isMobileDevice()}assets/index/Mobile.less (1)
9-29
: 很好的动画实现,建议考虑适配性移动端上升动画实现得很优雅,从底部滑入的效果对移动用户体验很友好。过渡时间设置为0.3秒也很合适,不会太快也不会太慢。
不过建议考虑在较小屏幕设备上的适配性,可能需要调整transform值或增加媒体查询以确保在各种移动设备上的一致体验。
src/Popup/index.tsx (2)
143-159
: 移动配置的处理逻辑简洁有效移动配置的检测和合并逻辑实现得很优雅。使用React.useMemo来缓存合并结果是一个很好的性能优化。代码结构清晰,易于理解和维护。
不过,可以考虑添加一些注释来解释合并策略,特别是当移动配置为空对象时的默认行为。
193-215
: 条件渲染逻辑清晰只在非移动模式下应用对齐样式的条件渲染逻辑很合理。代码结构清晰,逻辑分支明确。
不过,建议在isMobile判断前添加一个小注释,解释为什么移动模式下不需要这些对齐样式,以帮助其他开发者理解这个设计决策。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/__snapshots__/mobile.test.tsx.snap
is excluded by!**/*.snap
📒 Files selected for processing (11)
assets/index/Mobile.less
(1 hunks)docs/demos/mobile.md
(1 hunks)docs/examples/mobile.tsx
(1 hunks)src/Popup/Mask.tsx
(3 hunks)src/Popup/index.tsx
(8 hunks)src/hooks/useAction.ts
(1 hunks)src/hooks/useAlign.ts
(5 hunks)src/hooks/useWinClick.ts
(1 hunks)src/index.tsx
(11 hunks)src/interface.ts
(0 hunks)tests/mobile.test.tsx
(3 hunks)
💤 Files with no reviewable changes (1)
- src/interface.ts
🧰 Additional context used
🧬 Code Definitions (1)
src/hooks/useAlign.ts (1)
docs/examples/body-overflow.tsx (1)
open
(24-254)
🔇 Additional comments (37)
docs/demos/mobile.md (1)
1-8
: 文档结构清晰,符合要求这个新增的移动端演示文档结构简洁明了,正确使用了前置元数据设置标题和导航路径,并通过代码引用链接到了对应的示例代码。
src/hooks/useWinClick.ts (1)
6-9
: 新增的JSDoc注释提升了代码可读性为
useWinClick
函数添加的文档注释清晰地说明了函数的作用(在窗口点击时关闭弹出框)和返回值(处理弹出元素点击的函数)。这有助于其他开发者理解函数的用途和使用方式。src/Popup/Mask.tsx (3)
14-16
: 接口扩展合理,增加移动端支持在
MaskProps
接口中添加可选的mobile
属性,为组件提供了区分移动端和桌面端的能力,符合PR的主要目标。
26-28
: 正确解构新增属性从props中解构出
mobile
属性,保持了组件参数处理的一致性。
39-43
: 移动端样式类条件应用实现良好使用条件表达式
mobile && \
${prefixCls}-mobile-mask`来有选择地应用移动端特定样式类,这是React中处理条件类名的推荐做法。结合
classNames`库使用也更加灵活和可读。docs/examples/mobile.tsx (2)
1-4
: 导入依赖项正确正确导入了所需组件和样式,为示例做好了准备。
5-30
: 弹出位置配置全面
builtinPlacements
对象定义了多个方向的弹出位置,包括上、下、左、右及其变种(如左上、右下等),提供了丰富的布局选项。assets/index/Mobile.less (1)
31-53
: 遮罩层动画实现清晰,代码结构良好遮罩层的淡入淡出效果实现得很好,与主要内容的动画相匹配。代码结构清晰,命名规范统一。
这个实现与其他移动端组件库的常见模式一致,用户会感到熟悉和舒适。
src/Popup/index.tsx (5)
14-20
: MobileConfig接口设计合理移动配置接口的设计很清晰,提供了良好的类型安全性和灵活性。允许单独配置mask、motion和maskMotion非常实用,使开发者能够根据需要自定义移动端体验。
文档注释也很完善,帮助使用者理解每个属性的用途。
75-76
: PopupProps接口扩展恰当将mobile配置作为可选属性添加到PopupProps接口中是一个很好的设计决策,保持了向后兼容性,同时提供了移动端定制的能力。
183-190
: 移动模式下的偏移处理简化合理在移动模式下简化偏移样式是一个好的决定,因为移动弹窗通常是固定在屏幕底部的。这样的实现更符合移动端用户的期望。
247-249
: Mask组件的移动状态传递适当将合并后的mask、maskMotion和mobile状态传递给Mask组件是合适的。这确保了Mask组件能够正确响应移动配置。
273-275
: 类名条件添加实现得当基于移动状态条件性地添加mobile类名的实现很好。这为CSS样式提供了明确的钩子,便于应用移动特定的样式。
src/hooks/useAction.ts (4)
4-6
: 类型定义扩展合理添加'touch'类型到内部动作类型是一个好的改进,使代码能够更好地处理触摸事件。类型定义清晰,有助于提高代码的类型安全性。
16-16
: 函数返回类型更新适当更新函数返回类型以反映新的内部动作类型是必要的改变,保持了类型系统的一致性。
24-30
: 触摸事件处理逻辑优化改进的触摸事件处理逻辑更加简洁和优雅。现在基于动作类型而不是显式的移动检测来添加触摸动作,这是一个更灵活的方法。
这种方法可以更好地处理混合设备(如同时支持触摸和鼠标的设备),因为它关注的是交互类型而不是设备类型。
33-33
: 依赖数组更新正确useMemo依赖数组的更新准确反映了函数内部逻辑的变化,移除了不再使用的mobile参数。
tests/mobile.test.tsx (4)
4-4
: 导入类型增强了类型安全性引入TriggerProps类型是一个好的改进,增强了测试代码的类型安全性,使测试更加健壮。
39-55
: 触摸事件测试更新恰当触摸事件测试的更新很好地反映了代码中的变化。测试现在正确验证了触摸开始事件对弹出组件显示的影响,这对于确保移动功能正常工作至关重要。
测试清晰地验证了预期行为:触摸事件应该触发弹出显示,再次触摸应该隐藏弹出。
58-58
: 函数参数类型改进将getTrigger函数的参数类型从any改为Partial提高了类型安全性,减少了可能的错误,这是一个很好的改进。
74-114
: 移动配置测试覆盖全面新增的移动配置测试非常全面,涵盖了基本功能和自定义动画的情况。测试结构清晰,断言准确。
特别是对替换动画的测试很有价值,它确保了自定义动画能够正确应用于移动模式。这对于验证组件的可定制性非常重要。
src/index.tsx (11)
11-11
: 导入 MobileConfig 类型增强移动端支持引入
MobileConfig
类型使组件能够接受特定的移动端配置,这是一个很好的增强。
123-129
: JSDoc 注释清晰地解释了 mobile 属性的用途此注释很好地解释了
mobile
属性的使用场景和影响。标注为内部使用有助于防止不当使用。
204-204
: 使用简洁的布尔转换使用
!!mobile
将 mobile 属性转换为布尔值是一种简洁的实现方式。
254-266
: TypeScript 类型定义增强为事件处理器添加的详细类型定义提高了代码的类型安全性,有助于防止错误并提供更好的开发体验。
393-394
: 将移动状态传递给对齐钩子适当地将
isMobile
状态传递给useAlign
钩子,确保移动视图有不同的对齐行为。
527-542
: 添加触摸事件处理为移动设备添加专门的触摸事件处理是很好的做法。
touchedRef
的使用可以有效地跟踪触摸状态,避免触摸和鼠标事件之间的冲突。
582-584
: 实现忽略鼠标触发器函数这个函数很好地解决了触摸后鼠标事件的处理问题,防止了移动设备上可能出现的双重触发问题。
587-590
: 提取共享的鼠标进入回调将鼠标进入逻辑提取到单独的回调函数中是一个很好的重构,减少了代码重复并提高了可维护性。
626-639
: 增强鼠标离开事件处理更新后的鼠标离开处理逻辑,通过
ignoreMouseTrigger
检查避免触摸后的不必要鼠标事件响应,提高了移动体验。
736-739
: 移动端条件类名应用根据
isMobile
状态有条件地应用类名,确保移动设备上不使用对齐相关的类名,这很合理,因为移动视图的对齐逻辑不同。
780-782
: 将移动配置传递给 Popup 组件正确地将
mobile
配置传递给 Popup 组件,使移动特定的行为能够在弹出层中应用。src/hooks/useAlign.ts (5)
98-98
: 向 useAlign 钩子添加 mobile 参数适当地向
useAlign
钩子添加可选的mobile
参数,支持移动特定的对齐行为。
113-132
: 增加详细的 JSDoc 注释为
offsetInfo
状态中的每个字段添加清晰的 JSDoc 注释,大大提高了代码可读性和可维护性。
148-150
: 移动设备跳过滚动元素收集当检测到移动设备时返回空数组是合适的,因为移动视图通常有不同的滚动行为和布局需求。
175-176
: 移动设备跳过对齐逻辑为移动设备跳过复杂的对齐计算是一个很好的优化,因为移动布局通常采用不同的定位策略。
673-681
: 为对齐变量添加描述性注释添加的 JSDoc 注释清晰地解释了对齐计算中关键变量的作用,这有助于其他开发者理解这段复杂的计算逻辑。
New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #528 +/- ##
==========================================
+ Coverage 97.72% 97.77% +0.05%
==========================================
Files 13 13
Lines 790 809 +19
Branches 227 245 +18
==========================================
+ Hits 772 791 +19
Misses 18 18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.eslintrc.js (1)
13-17
: 添加了新的 TypeScript ESLint 规则配置新增了5个 TypeScript 特定的 ESLint 规则并将它们禁用(设置为 0)。禁用这些规则可能会允许代码库中出现一些不符合最佳实践的模式,但可能是为了适应移动端配置所需的特定代码模式。
这些规则包括:
@typescript-eslint/consistent-indexed-object-style
:确保索引对象使用一致的风格@typescript-eslint/no-parameter-properties
:禁止在类构造函数中使用参数属性@typescript-eslint/ban-types
:禁止使用某些可能有问题的类型(如{}
,Object
等)@typescript-eslint/type-annotation-spacing
:强制类型注解周围的空格一致性@typescript-eslint/no-throw-literal
:确保只抛出 Error 对象建议在项目稳定后重新评估这些规则的禁用是否必要,以保持代码质量。
docs/examples/mobile.tsx (2)
32-54
: 触发文本与交互行为不匹配组件的触发文本是"Click Me",但实际配置的交互方式却是
hover
。这可能会让使用者感到困惑,因为文本暗示需要点击,而实际上是悬停触发。建议修改为以下两种方案之一:
- action={['hover']} + action={['click']}或:
- <span>Click Me</span> + <span>Hover Me</span>
32-66
: 缺少示例说明注释作为一个演示移动端功能的关键示例,建议添加注释来解释移动配置的目的和效果,以及它与非移动配置的区别。这将帮助开发者更好地理解这个新功能。
建议在组件顶部添加类似以下的注释:
/** * 移动端配置示例 * * 此示例展示了Trigger组件的移动端特定配置,包括: * 1. 启用遮罩层 (mask) * 2. 自定义弹出层动画 (raise) * 3. 自定义遮罩层动画 (fade) * * 在移动设备上,这些配置会提供更适合触摸交互的用户体验。 */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.eslintrc.js
(1 hunks)docs/examples/mobile.tsx
(1 hunks)package.json
(1 hunks)src/index.tsx
(11 hunks)src/interface.ts
(0 hunks)
💤 Files with no reviewable changes (1)
- src/interface.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/index.tsx
🔇 Additional comments (3)
package.json (1)
64-64
: 锁定开发依赖版本是一个明智的选择将TypeScript版本从
^5.1.6
更改为~5.1.6
是个良好实践。这种变更将只允许补丁版本更新(5.1.7、5.1.8等),而不是次要版本更新,可以减少因TypeScript版本变化带来的不兼容风险,确保构建环境的稳定性。docs/examples/mobile.tsx (2)
1-30
: 良好的引入和基础配置设置导入了必要的依赖并正确配置了
builtinPlacements
对象,为弹出层定义了全面的位置选项。这为示例提供了完整的弹出位置支持。
55-59
: 移动端配置已正确实现之前被注释掉的移动端配置现在已启用,这与PR的目标一致。当前配置为移动设备提供了遮罩层和动画效果。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces mobile configuration support and interaction improvements across the Trigger component and related modules. Key changes include updated mobile tests, revised mobile configuration handling in the Trigger and Popup components, and accompanying documentation and demo updates.
Reviewed Changes
Copilot reviewed 11 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tests/mobile.test.tsx | Updated mobile interaction tests and event handling for touch events |
src/interface.ts | Removed the legacy MobileConfig interface |
src/index.tsx | Modified mobile configuration handling and event binding within the Trigger generator |
src/hooks/useWinClick.ts | Added documentation for window click events handling for mobile |
src/hooks/useAlign.ts | Included mobile flag to conditionally bypass alignment when in mobile mode |
src/hooks/useAction.ts | Adjusted action sets to include touch events for mobile configurations |
src/Popup/index.tsx | Updated Popup to use a mobile configuration for mask and motion while adjusting offset style |
src/Popup/Mask.tsx | Added mobile-dependent className to Mask for styling purposes |
docs/examples/mobile.tsx | Provided a new mobile interaction example |
docs/demos/mobile.md | Added mobile demo documentation |
.eslintrc.js | Updated ESLint configuration with new TypeScript rules |
Files not reviewed (3)
- assets/index/Mobile.less: Language not supported
- package.json: Language not supported
- tests/snapshots/mobile.test.tsx.snap: Language not supported
Comments suppressed due to low confidence (1)
src/index.tsx:204
- [nitpick] Consider renaming the destructured 'mobile' prop (e.g., to 'mobileConfig') and then using a separate boolean variable 'isMobile' for clarity. This can help avoid confusion between a configuration object and its boolean evaluation.
const isMobile = !!mobile;
* @private Bump fixed position at bottom in mobile. | ||
* Will replace the config of root props. | ||
* This will directly trade as mobile view which will not check what real is. | ||
* This is internal usage currently, do not use in your prod. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The documentation for the mobile prop contains unclear phrasing. Consider rephrasing to something like: 'This private prop forces mobile view by overriding the root configuration, bypassing actual mobile detection.'
* @private Bump fixed position at bottom in mobile. | |
* Will replace the config of root props. | |
* This will directly trade as mobile view which will not check what real is. | |
* This is internal usage currently, do not use in your prod. | |
* This private prop forces mobile view by overriding the root configuration, | |
* bypassing actual mobile detection. This is for internal use only and should | |
* not be used in production. |
Copilot uses AI. Check for mistakes.
This is not a API to support mobile detection directly. Which need wrap component to decide whether to use mobile config.
Summary by CodeRabbit
新功能
样式
文档
重构
测试