Correct OnDismiss event calling in BitModal component (#10016)#10017
Correct OnDismiss event calling in BitModal component (#10016)#10017msynk merged 3 commits intobitfoundation:developfrom
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes update the modal component by introducing a new private method Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Interaction
participant Modal as BitModal Component
participant Callback as ModalParameters.OnDismiss
UI->>Modal: Set IsOpen = false
Modal-->>Modal: Invoke OnSetIsOpen()
Modal->>Callback: Execute OnDismiss() if IsOpen is false
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Modal/BitModal.razor.cs(2 hunks)
🔇 Additional comments (2)
src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Modal/BitModal.razor.cs (2)
83-85: LGTM! Good use of [CallOnSet] to centralize dismissal logic.The addition of [CallOnSet] ensures consistent behavior by automatically triggering OnDismiss whenever the modal is closed, regardless of how IsOpen is set to false.
211-220:Details
✅ Verification successful
Verify the removal of OnDismiss from HandleOnOverlayClick.
The changes correctly remove the direct OnDismiss invocation from HandleOnOverlayClick, as it's now handled through OnSetIsOpen when IsOpen is set to false. This eliminates potential duplicate calls to OnDismiss.
Run the following script to ensure there are no remaining direct calls to OnDismiss:
🏁 Script executed:
#!/bin/bash # Description: Search for direct invocations of OnDismiss # Expect: Only OnSetIsOpen should invoke OnDismiss rg "OnDismiss.InvokeAsync" -A 2 -B 2Length of output: 4700
Direct OnDismiss Call Removed from HandleOnOverlayClick
The update successfully eliminates the direct invocation of
OnDismiss.InvokeAsyncfrom theHandleOnOverlayClickmethod inBitModal.razor.cs, ensuring that dismissal is now uniformly handled through the centralizedOnSetIsOpenlogic. Our search confirms that while other components continue to useOnDismiss.InvokeAsyncas part of their dismissal flow, the modal component under review no longer misfires this event via the overlay click.
- BitModal.razor.cs: The
HandleOnOverlayClickmethod has been updated to remove the direct call toOnDismiss.InvokeAsync.- Remaining calls to
OnDismiss.InvokeAsyncin the central dismissal logic (and in other components) appear intentional and consistent with the design improvements.Overall, the changes align with the PR objectives by eliminating duplicate invocation and centralizing the dismissal behavior.
2 participants
closes #10016
Summary by CodeRabbit