Conversation
Reviewer's GuideAdds paste handling to the IpAddress component’s IPv4 input cells, allowing users to paste an IP-like string which is parsed, sanitized, and distributed across the four octet fields, and slightly adjusts key handling conditions to better support composed events. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
- The
else if (e.composed || e.key === 'Delete' || 'Tab'...)branch looks suspicious:e.composedis a boolean unrelated to keyboard shortcuts, and the branch body is empty, so it’s unclear what behavior is intended here (e.g., paste via Ctrl+V?)—consider either removing this condition or wiring it to an explicit use case. - In the paste handler, you currently update the visible
.ipv4-cellvalues andip.prevValuesbut don’t appear to trigger any change/blur logic used elsewhere in the component; if other logic depends on those events or a consolidated value, consider reusing the same update path to keep behavior consistent with manual input.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `else if (e.composed || e.key === 'Delete' || 'Tab'...)` branch looks suspicious: `e.composed` is a boolean unrelated to keyboard shortcuts, and the branch body is empty, so it’s unclear what behavior is intended here (e.g., paste via Ctrl+V?)—consider either removing this condition or wiring it to an explicit use case.
- In the paste handler, you currently update the visible `.ipv4-cell` values and `ip.prevValues` but don’t appear to trigger any change/blur logic used elsewhere in the component; if other logic depends on those events or a consolidated value, consider reusing the same update path to keep behavior consistent with manual input.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/IpAddress/IpAddress.razor.js:76` </location>
<code_context>
selectCell(el, index - 1)
}
- else if (e.key === 'Delete' || e.key === 'Tab' || e.key === 'ArrowLeft' || e.key === 'ArrowRight') {
+ else if (e.composed || e.key === 'Delete' || e.key === 'Tab' || e.key === 'ArrowLeft' || e.key === 'ArrowRight') {
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `e.composed` here may unintentionally swallow most key events.
Because many keyboard events have `composed === true` by default, this condition will match far more often than intended and prevent the final `else` from ever handling normal key input. If you’re trying to special-case IME/composition or modifier shortcuts, use something more specific (e.g. `e.isComposing`, `e.ctrlKey`, etc.) instead of `e.composed`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7276 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 745 745
Lines 32621 32621
Branches 4522 4522
=========================================
Hits 32621 32621
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds paste functionality to the IpAddress component, allowing users to paste IP addresses from their clipboard into the input fields. The implementation includes a new paste event handler that parses clipboard text and populates the IP address cells.
Key Changes:
- Added a paste event handler that parses IP address strings from clipboard data
- Modified the keydown event handler to include
e.composedin the allowed keys condition
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| EventHandler.on(c, 'paste', e => { | ||
| e.preventDefault(); | ||
| const raw = (e.clipboardData || window.clipboardData)?.getData('text') ?? ''; | ||
| if (!raw) { | ||
| return; | ||
| } | ||
| const parts = raw.replace(/[^\d.]/g, '').split('.').filter(p => p.length); | ||
| const cells = el.querySelectorAll(".ipv4-cell"); | ||
| let pos = 0; | ||
| parts.forEach(p => { | ||
| if (pos > 3) { | ||
| return; | ||
| } | ||
| const num = Math.max(0, Math.min(255, parseInt(p, 10) || 0)); | ||
| cells[pos].value = num.toString(); | ||
| ip.prevValues[pos] = cells[pos].value; | ||
| pos++; | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The paste event handler always starts pasting from position 0, ignoring which cell the user is actually focused on. When a user pastes "192.168" while focused on the second cell, it overwrites the first two cells instead of filling from the second cell onward.
Consider using the index parameter (available in the forEach scope) to start pasting from the currently focused cell:
let pos = index;
parts.forEach(p => {
if (pos > 3) {
return;
}
const num = Math.max(0, Math.min(255, parseInt(p, 10) || 0));
cells[pos].value = num.toString();
ip.prevValues[pos] = cells[pos].value;
pos++;
});| selectCell(el, index - 1) | ||
| } | ||
| else if (e.key === 'Delete' || e.key === 'Tab' || e.key === 'ArrowLeft' || e.key === 'ArrowRight') { | ||
| else if (e.composed || e.key === 'Delete' || e.key === 'Tab' || e.key === 'ArrowLeft' || e.key === 'ArrowRight') { |
There was a problem hiding this comment.
The addition of e.composed to this condition is incorrect. The composed property is a boolean that indicates whether the event can bubble across the shadow DOM boundary, not whether the event is from IME composition. This will allow all composed events (which is most standard events) to bypass validation, potentially allowing invalid input.
If the intent is to allow IME composition input, this is already handled by the separate compositionend event handler at line 25. The e.composed check should be removed.
| else if (e.composed || e.key === 'Delete' || e.key === 'Tab' || e.key === 'ArrowLeft' || e.key === 'ArrowRight') { | |
| else if (e.key === 'Delete' || e.key === 'Tab' || e.key === 'ArrowLeft' || e.key === 'ArrowRight') { |
| const parts = raw.replace(/[^\d.]/g, '').split('.').filter(p => p.length); | ||
| const cells = el.querySelectorAll(".ipv4-cell"); | ||
| let pos = 0; | ||
| parts.forEach(p => { | ||
| if (pos > 3) { | ||
| return; | ||
| } | ||
| const num = Math.max(0, Math.min(255, parseInt(p, 10) || 0)); | ||
| cells[pos].value = num.toString(); | ||
| ip.prevValues[pos] = cells[pos].value; | ||
| pos++; | ||
| }); |
There was a problem hiding this comment.
The regex /[^\d.]/g allows multiple consecutive dots and doesn't validate IP address format. For example, "192...168" would be parsed as ["192", "168"], silently ignoring the malformed input. Consider adding validation or using a more robust parsing approach that validates IP address format before processing.
| const parts = raw.replace(/[^\d.]/g, '').split('.').filter(p => p.length); | |
| const cells = el.querySelectorAll(".ipv4-cell"); | |
| let pos = 0; | |
| parts.forEach(p => { | |
| if (pos > 3) { | |
| return; | |
| } | |
| const num = Math.max(0, Math.min(255, parseInt(p, 10) || 0)); | |
| cells[pos].value = num.toString(); | |
| ip.prevValues[pos] = cells[pos].value; | |
| pos++; | |
| }); | |
| // Validate IPv4 address format: four octets, each 0-255, separated by dots | |
| const ipv4Regex = /^(25[0-5]|2[0-4]\d|1\d{2}|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d{2}|[1-9]?\d)){3}$/; | |
| if (!ipv4Regex.test(raw.trim())) { | |
| // Invalid IP address format; ignore paste | |
| return; | |
| } | |
| const parts = raw.trim().split('.'); | |
| const cells = el.querySelectorAll(".ipv4-cell"); | |
| for (let pos = 0; pos < 4; pos++) { | |
| cells[pos].value = parts[pos]; | |
| ip.prevValues[pos] = cells[pos].value; | |
| } |
| } | ||
| }) | ||
|
|
||
| EventHandler.on(c, 'paste', e => { |
There was a problem hiding this comment.
The new 'paste' event handler is not being cleaned up in the dispose function (lines 115-125). This creates a memory leak as the event listener will persist after the component is disposed.
The dispose function should include:
EventHandler.off(c, 'paste')* feat(IpAddress): support paste function * refactor: 代码格式化 # Conflicts: # src/BootstrapBlazor/Components/IpAddress/IpAddress.razor.js
* feat: 增加 Key 参数 * refactor: 使用回调方法 GetKeyByITem 获得行 Key 值 * chore: bump version 9.6.5 * feat: 增加 PdfOptions 配置项 * chore: bump version 9.6.6 * refactor: 调整 IconTemplate 优先级别 * feat: 增加 ActionButtonTemplate 模板 * refactor: 增加条件 * chore: bump version 9.6.7 * feat(IpAddress): support paste function (#7276) * feat(IpAddress): support paste function * refactor: 代码格式化 # Conflicts: # src/BootstrapBlazor/Components/IpAddress/IpAddress.razor.js * chore: bump version 9.6.8 * feat(IpAddress): trigger ValueChanged on paste event (#7280) * refactor: 增加 JSInvoke 能力 * refactor: 更改样式 * refactor: 增加客户端更改 IP 回调方法 * test: 更新单元测试 * chore: bump version 10.1.3 # Conflicts: # src/BootstrapBlazor/BootstrapBlazor.csproj # src/BootstrapBlazor/Components/IpAddress/IpAddress.razor.scss # test/UnitTest/Components/IpAddressTest.cs * chore: bump version 9.6.9 * chore: 支持 Interop 参数 * chore: bump version 9.6.10 * feat: 增加 OnBeforeTreeItemClick 方法 * chore: bump version 9.6.11 * refactor: 兼容嵌套问题 * chore: bump version 9.6.12 * feat: 支持端口 * chore: bump version 9.6-13-beta01 * chore: bump version 9.6.13 * feat(ZipArchiveService): add ArchiveDirectoryAsync method # Conflicts: # src/BootstrapBlazor/Services/DefaultZipArchiveService.cs # src/BootstrapBlazor/Services/IZipArchiveService.cs * test: 增加单元测试 # Conflicts: # test/UnitTest/Services/ZipArchiveServiceTest.cs * chore: bump version 9.6.14 * refactor: 增加 Key 参数 * refactor: 取消冗余代码 * revert: 撤销更改 * refactor: 移动 SetKey 位置 * refactor: 调整序号 * test: 增加单元测试
* feat: 增加 Key 参数 * refactor: 使用回调方法 GetKeyByITem 获得行 Key 值 * chore: bump version 9.6.5 * feat: 增加 PdfOptions 配置项 * chore: bump version 9.6.6 * refactor: 调整 IconTemplate 优先级别 * feat: 增加 ActionButtonTemplate 模板 * refactor: 增加条件 * chore: bump version 9.6.7 * feat(IpAddress): support paste function (#7276) * feat(IpAddress): support paste function * refactor: 代码格式化 # Conflicts: # src/BootstrapBlazor/Components/IpAddress/IpAddress.razor.js * chore: bump version 9.6.8 * feat(IpAddress): trigger ValueChanged on paste event (#7280) * refactor: 增加 JSInvoke 能力 * refactor: 更改样式 * refactor: 增加客户端更改 IP 回调方法 * test: 更新单元测试 * chore: bump version 10.1.3 # Conflicts: # src/BootstrapBlazor/BootstrapBlazor.csproj # src/BootstrapBlazor/Components/IpAddress/IpAddress.razor.scss # test/UnitTest/Components/IpAddressTest.cs * chore: bump version 9.6.9 * chore: 支持 Interop 参数 * chore: bump version 9.6.10 * feat: 增加 OnBeforeTreeItemClick 方法 * chore: bump version 9.6.11 * refactor: 兼容嵌套问题 * chore: bump version 9.6.12 * feat: 支持端口 * chore: bump version 9.6-13-beta01 * chore: bump version 9.6.13 * feat(ZipArchiveService): add ArchiveDirectoryAsync method # Conflicts: # src/BootstrapBlazor/Services/DefaultZipArchiveService.cs # src/BootstrapBlazor/Services/IZipArchiveService.cs * test: 增加单元测试 # Conflicts: # test/UnitTest/Services/ZipArchiveServiceTest.cs * chore: bump version 9.6.14 * refactor: 增加 Key 参数 * refactor: 取消冗余代码 * revert: 撤销更改 * refactor: 移动 SetKey 位置 * refactor: 调整序号 * test: 增加单元测试 * refactor: 更新代码 * refactor: 重构代码 * refactor: 重构代码 * refactor: 重构代码 * refactor: 重构代码 * refactor: 更新代码
Link issues
fixes #7275
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add IP address input enhancements to better handle clipboard-driven input and navigation behavior.
New Features:
Enhancements: