-
Notifications
You must be signed in to change notification settings - Fork 43
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
fix: autofocus input from dialog no effect #43
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #43 +/- ##
==========================================
- Coverage 92.34% 92.00% -0.35%
==========================================
Files 5 5
Lines 222 225 +3
Branches 79 80 +1
==========================================
+ Hits 205 207 +2
- Misses 17 18 +1 ☔ View full report in Codecov by Sentry. |
Walkthrough此次更改引入了一个新的对话框组件样式文件 Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
@@ -30,3 +30,4 @@ | |||
} | |||
} | |||
} | |||
@import './autofocus.less'; |
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.
Should not import modal styles, it is for debug only.
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
🧹 Outside diff range and nitpick comments (9)
docs/examples/autofocus.tsx (4)
1-6
: 建议添加样式类型声明导入为了保持类型安全,建议为 less 模块添加类型声明。
添加以下类型声明:
+declare module '*.less' { + const classes: { [key: string]: string }; + export default classes; +}
7-9
: 建议使用更具描述性的状态变量名称
vis
这个变量名称不够直观,建议改为更具描述性的名称。建议修改如下:
-const [vis, setVis] = React.useState(false); +const [isDialogVisible, setIsDialogVisible] = React.useState(false);
19-24
: 建议改进对话框标题的国际化处理当前对话框标题直接使用中文硬编码,建议使用国际化处理或至少使用英文作为演示。
建议修改如下:
-title={<div>第二个弹框</div>} +title={<div>Dialog with AutoFocus Input</div>}
16-18
: 建议优化按钮文本样式按钮文本中包含了硬编码的空格字符
{' '}
,建议使用 CSS 样式来控制间距。建议修改如下:
-toggle autoFocus input{' '} +toggle autoFocus input同时添加适当的 CSS 样式:
button { margin-right: 8px; }assets/autofocus.less (4)
1-16
: 建议添加 ARIA 属性以提高可访问性对话框组件应该包含适当的 ARIA 属性,以确保屏幕阅读器可以正确识别和朗读对话框内容。
建议添加以下样式:
.rc-dialog { position: relative; width: auto; margin: 10px; + role: dialog; + aria-modal: true; }
60-77
: 建议使用 CSS 变量提高可维护性当前实现中的颜色值和间距值都是硬编码的,建议使用 CSS 变量来统一管理这些值。
建议添加以下变量定义:
+ @dialog-border-color: #e9e9e9; + @dialog-padding: 20px; &-header { - padding: 13px 20px 14px 20px; + padding: 13px @dialog-padding 14px @dialog-padding; border-radius: 5px 5px 0 0; background: #fff; color: #666; - border-bottom: 1px solid #e9e9e9; + border-bottom: 1px solid @dialog-border-color; } &-body { - padding: 20px; + padding: @dialog-padding; } &-footer { - border-top: 1px solid #e9e9e9; - padding: 10px 20px; + border-top: 1px solid @dialog-border-color; + padding: 10px @dialog-padding; text-align: right; border-radius: 0 0 5px 5px; }
79-126
: 建议优化动画性能当前动画实现可以通过以下方式优化性能:
- 使用
transform
替代直接修改opacity
- 添加
will-change
属性- 使用
transform3d
触发 GPU 加速建议进行如下修改:
.effect() { animation-duration: 0.3s; animation-fill-mode: both; + will-change: transform, opacity; } &-content { position: relative; background-color: #ffffff; border: none; border-radius: 6px 6px; background-clip: padding-box; + transform: translateZ(0); }
129-134
: 建议使用更灵活的响应式布局当前的媒体查询使用固定宽度值,可能不适合所有场景。建议使用相对单位和更多的断点来实现更好的响应式效果。
建议修改如下:
+ @screen-sm: 576px; + @screen-md: 768px; + @screen-lg: 992px; - @media (min-width: 768px) { + @media (min-width: @screen-sm) { .rc-dialog { - width: 600px; + width: 90%; + max-width: 500px; margin: 30px auto; } } + @media (min-width: @screen-md) { + .rc-dialog { + max-width: 600px; + } + } + @media (min-width: @screen-lg) { + .rc-dialog { + max-width: 800px; + } + }src/Input.tsx (1)
Line range hint
1-300
: 建议完善文档说明为了帮助其他开发者更好地理解和使用自动聚焦功能,建议在组件文档中补充以下内容:
- 对话框中使用自动聚焦的示例代码
- 自动聚焦的触发时机说明
- 与禁用状态的交互说明
需要我帮助编写相关文档说明吗?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
assets/autofocus.less
(1 hunks)assets/index.less
(1 hunks)docs/demo/autofocus.md
(1 hunks)docs/examples/autofocus.tsx
(1 hunks)package.json
(1 hunks)src/Input.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- assets/index.less
- docs/demo/autofocus.md
🔇 Additional comments (3)
docs/examples/autofocus.tsx (1)
25-25
: 验证自动聚焦功能在对话框中的行为
需要确保在对话框打开时输入框能够正确获得焦点。
✅ Verification successful
autoFocus 属性已正确实现,无需额外验证
从代码分析结果可以看到,Input 组件已经在 src/Input.tsx
中正确处理了 autoFocus 属性:
- 组件内部通过 useEffect 监听 autoFocus 属性的变化
- 当 autoFocus 为 true 时会自动调用 focus() 方法
- 示例代码中的用法符合组件的设计预期
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查 Input 组件中是否正确处理了 autoFocus
ast-grep --pattern 'useEffect($_) {
$$$
focus()
$$$
}'
Length of output: 62
Script:
#!/bin/bash
# 搜索 Input 组件的实现
rg -A 10 "const Input"
# 搜索包含 autoFocus 属性的相关代码
rg -A 5 "autoFocus"
# 搜索 focus 相关的实现
ast-grep --pattern 'const $_ = useRef($$$)'
Length of output: 1761
package.json (1)
72-72
: 依赖项添加正确!
rc-dialog 作为开发依赖项添加是合适的,因为它仅用于演示和测试目的。
让我们验证一下 rc-dialog 的版本兼容性:
✅ Verification successful
Let me generate one more verification to check the project's React version before making the final response.
Let me try one more time to find React dependencies, checking both dependencies and devDependencies:
rc-dialog 依赖版本完全兼容!
经验证,[email protected] 与项目完全兼容:
- rc-dialog 要求 React >= 16.9.0,而项目使用 React 18.0.0
- 未发现任何安全漏洞
- 作为开发依赖使用是合适的
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查 rc-dialog 的版本兼容性
# 获取 rc-dialog 的 package.json 信息
curl -s https://registry.npmjs.org/rc-dialog/9.1.0 | jq -r '.peerDependencies'
# 检查是否有已知问题
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "rc-dialog") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 474
Script:
#!/bin/bash
# 检查项目的 React 版本
cat package.json | jq -r '.dependencies.react, .dependencies["react-dom"]'
Length of output: 86
Script:
#!/bin/bash
# 检查项目的 React 版本(包括开发依赖)
cat package.json | jq -r '.dependencies.react, .dependencies["react-dom"], .devDependencies.react, .devDependencies["react-dom"]'
Length of output: 157
src/Input.tsx (1)
102-107
: 实现方案正确且简洁!
新增的 useEffect 钩子能够正确处理 autoFocus 属性,使其在对话框场景下也能正常工作。
建议添加以下测试用例:
- 对话框打开时自动聚焦
- 对话框关闭后重新打开时的聚焦行为
- 禁用状态下的自动聚焦处理
运行以下脚本验证对话框集成:
✅ Verification successful
自动聚焦功能已正确实现且经过验证
通过检查代码库,我们发现:
- 示例文件
docs/examples/autofocus.tsx
中已包含对话框场景的自动聚焦测试用例 - Input 组件中的 autoFocus 实现简洁且正确
- 已有完整的聚焦相关测试文件
tests/focus.test.tsx
无需添加额外的测试用例,因为:
- 示例代码已经覆盖了对话框打开时的自动聚焦场景
- 基础组件 BaseInput 中已包含了完整的聚焦行为测试
- 当前实现遵循了 React 的最佳实践,使用 useEffect 正确处理 autoFocus 属性
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证对话框组件中的自动聚焦实现
# 检查测试文件中是否包含相关测试用例
rg -l "autoFocus.*dialog" --type ts --type tsx
# 检查是否有其他组件使用类似实现
ast-grep --pattern 'useEffect(() => {
if ($focus) {
$$$
}
}, [$focus])'
Length of output: 158
Script:
#!/bin/bash
# 检查测试文件中是否包含相关测试用例
rg -l "autoFocus.*dialog" --type typescript --type javascriptreact --type typescriptreact
# 检查对话框组件中的自动聚焦相关代码
rg "autoFocus.*dialog" -A 5 --type typescript --type javascriptreact --type typescriptreact
# 检查是否有其他组件使用类似实现
ast-grep --pattern 'useEffect(() => {
if ($focus) {
$$$
}
}, [$focus])'
# 查找对话框相关的测试文件
fd ".*dialog.*test.*" -e ts -e tsx -e js -e jsx
# 查找Input组件的测试文件
fd ".*input.*test.*" -e ts -e tsx -e js -e jsx
Length of output: 461
Script:
#!/bin/bash
# 检查Input组件的测试文件内容
cat tests/BaseInput.test.tsx
# 使用基本的grep查找对话框和自动聚焦相关代码
find . -type f -name "*.tsx" -o -name "*.ts" | xargs grep -l "autoFocus"
find . -type f -name "*.tsx" -o -name "*.ts" | xargs grep -l "dialog"
# 检查是否有其他组件使用useEffect和focus
find . -type f -name "*.tsx" -o -name "*.ts" | xargs grep -A 5 "useEffect.*focus"
Length of output: 11493
Script:
#!/bin/bash
# 检查自动聚焦示例文件的内容
cat docs/examples/autofocus.tsx
# 检查Input组件中的autoFocus相关代码
find . -type f -name "Input.tsx" | xargs grep -A 10 "autoFocus"
# 查找所有包含focus的测试用例
find . -type f -name "*test*" -exec grep -l "focus" {} \;
Length of output: 3230
&-close { | ||
cursor: pointer; | ||
border: 0; | ||
background: transparent; | ||
font-size: 21px; | ||
position: absolute; | ||
right: 20px; | ||
top: 12px; | ||
font-weight: 700; | ||
line-height: 1; | ||
color: #000; | ||
text-shadow: 0 1px 0 #fff; | ||
filter: alpha(opacity=20); | ||
opacity: .2; | ||
text-decoration: none; | ||
|
||
&-x:after { | ||
content: '×' | ||
} | ||
|
||
&:hover { | ||
opacity: 1; | ||
filter: alpha(opacity=100); | ||
text-decoration: none; | ||
} | ||
} |
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.
🛠️ Refactor suggestion
优化关闭按钮的实现
关闭按钮的当前实现存在以下问题:
- 缺少键盘访问支持
- 使用伪元素添加 '×' 符号不利于国际化
- 缺少 ARIA 标签
建议进行如下修改:
&-close {
cursor: pointer;
border: 0;
background: transparent;
font-size: 21px;
position: absolute;
right: 20px;
top: 12px;
font-weight: 700;
line-height: 1;
color: #000;
text-shadow: 0 1px 0 #fff;
filter: alpha(opacity=20);
opacity: .2;
text-decoration: none;
+ outline: none;
+ aria-label: "关闭";
- &-x:after {
- content: '×'
- }
+ &:focus {
+ opacity: 1;
+ box-shadow: 0 0 0 2px rgba(0, 0, 0, 0.1);
+ }
&:hover {
opacity: 1;
filter: alpha(opacity=100);
text-decoration: none;
}
}
Committable suggestion skipped: line range outside the PR's diff.
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
📝 更新日志
☑️ 请求合并前的自查清单
🚀 概述
copilot:summary
🔍 实现细节
copilot:walkthrough
Summary by CodeRabbit
新功能
文档
依赖更新