feat(SelectTable): add IsMultipleSelect parameter#7629
Conversation
|
Thanks for your PR, @Tony-ST0754. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Reviewer's GuideAdds JS interop initialization and a CSS marker class to BootstrapInputNumber to correctly toggle between numeric and formatted (thousand-separated) display on focus/blur, fixing the thousand-separator formatting issue. Sequence diagram for BootstrapInputNumber focus/blur thousand-separator formattingsequenceDiagram
actor User
participant Browser
participant BootstrapInputNumber_TValue as BootstrapInputNumber_TValue
participant BootstrapInputJs as BootstrapInput_razor_js
User->>Browser: Load page with BootstrapInputNumber
Browser->>BootstrapInputNumber_TValue: Component initialization
BootstrapInputNumber_TValue->>BootstrapInputJs: init(id, Interop, TriggerFormatValue)
User->>Browser: Focus input
Browser->>BootstrapInputJs: focus event
BootstrapInputJs->>BootstrapInputJs: Check type is text and class contains input-number
BootstrapInputJs->>Browser: Set type to number
BootstrapInputJs->>Browser: Remove thousand separators from value
BootstrapInputJs->>BootstrapInputNumber_TValue: invokeMethodAsync(TriggerFormatValue, true)
BootstrapInputNumber_TValue->>BootstrapInputNumber_TValue: TriggerFormatValue(true)
BootstrapInputNumber_TValue->>BootstrapInputNumber_TValue: CurrentValueAsString = InternalFormat(Value)
BootstrapInputNumber_TValue->>Browser: StateHasChanged rerenders input
User->>Browser: Blur input
Browser->>BootstrapInputJs: blur event
BootstrapInputJs->>BootstrapInputJs: Check type is number and class contains input-number
BootstrapInputJs->>Browser: Set type to text
BootstrapInputJs->>BootstrapInputNumber_TValue: invokeMethodAsync(TriggerFormatValue, false)
BootstrapInputNumber_TValue->>BootstrapInputNumber_TValue: TriggerFormatValue(false)
BootstrapInputNumber_TValue->>BootstrapInputNumber_TValue: CurrentValueAsString = GetFormatString(Value)
BootstrapInputNumber_TValue->>Browser: StateHasChanged rerenders formatted value
Updated class diagram for BootstrapInputNumber thousand-separator behaviorclassDiagram
class BootstrapInputNumber_TValue {
<<partial>>
+string InputClassString
+Task InvokeInitAsync()
+Task TriggerFormatValue(bool focus)
}
class CssBuilder {
+CssBuilder Default(string value)
+CssBuilder AddClass(string value)
+CssBuilder AddClass(string value, bool condition)
+CssBuilder AddClassFromAttributes(object additionalAttributes)
}
BootstrapInputNumber_TValue --> CssBuilder : builds InputClassString
class BootstrapModuleAutoLoader_Attribute {
+BootstrapModuleAutoLoader_Attribute(string path)
+bool JSObjectReference
}
BootstrapModuleAutoLoader_Attribute <|.. BootstrapInputNumber_TValue
class BootstrapInput_razor_js {
+void init(string id, object invoke, string method)
+void focus(string id)
}
BootstrapInputNumber_TValue ..> BootstrapInput_razor_js : JSInterop init(id, Interop, TriggerFormatValue)
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 - I've left some high level feedback:
- In
BootstrapInput.razor.js, usinge.target.getAttribute('class').includes('input-number')is brittle; prefere.target.classList?.contains('input-number')to avoid false positives and issues whenclassis null or large. - The
inithandler attaches focus/blur handlers but never removes them; consider wiring cleanup (e.g., via a dispose function or existing module pattern) to avoid leaking handlers when components are re-rendered or removed. - The
InvokeInitAsyncoverride inBootstrapInputNumberreplaces the base behavior entirely; if the base class has important initialization logic, consider awaitingbase.InvokeInitAsync()before or after your JS interop call.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `BootstrapInput.razor.js`, using `e.target.getAttribute('class').includes('input-number')` is brittle; prefer `e.target.classList?.contains('input-number')` to avoid false positives and issues when `class` is null or large.
- The `init` handler attaches focus/blur handlers but never removes them; consider wiring cleanup (e.g., via a dispose function or existing module pattern) to avoid leaking handlers when components are re-rendered or removed.
- The `InvokeInitAsync` override in `BootstrapInputNumber` replaces the base behavior entirely; if the base class has important initialization logic, consider awaiting `base.InvokeInitAsync()` before or after your JS interop call.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 #7629 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 749 749
Lines 33017 33105 +88
Branches 4581 4593 +12
=========================================
+ Hits 33017 33105 +88
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:
|
BootstrapInputNumber格式化千分位不显示的问题,给组件添加一个class组件特殊标识,然后通过js回调显示,不知道这个合不合适
e3eccaa to
8446f14
Compare
This reverts commit 8446f14.
Link issues
fixes #7628
fixes #7633
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Fix BootstrapInputNumber thousand-separator formatting not displaying correctly by integrating JS interop to toggle numeric/text input on focus and blur.
Bug Fixes:
Enhancements: