add more rules for dialog parents and children#5945
Merged
Conversation
CodSpeed Performance ReportMerging #5945 will not alter performanceComparing Summary
|
Contributor
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR strengthens the Dialog component hierarchy by adding parent-child validation rules to ensure proper component structure.
Key Changes:
- Added
ClassVarimport and proper type annotations for validation rules - Added
_valid_childrentoDialogRoot(allowsDialogTriggerandDialogPortal) - Added
_valid_parentsconstraints toDialogPortal,DialogOverlay, andDialogContent - Fixed
DialogTrigger._valid_parentsto use properClassVartype annotation
Issues Found:
DialogPortalis missing_valid_childrendefinition - should specifyDialogOverlayandDialogContentas valid children to complete the validation hierarchy
Confidence Score: 4/5
- This PR is safe to merge with one incomplete validation rule that should be addressed.
- The changes properly implement parent-child validation for Dialog components using consistent patterns found elsewhere in the codebase. The type annotations are correct and follow best practices. However,
DialogPortalis missing_valid_childrendefinition, which means the validation hierarchy is incomplete - this won't break existing functionality but reduces the effectiveness of the validation system. - Pay attention to
reflex/components/radix/primitives/dialog.py- theDialogPortalclass needs_valid_childrendefined.
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| reflex/components/radix/primitives/dialog.py | 4/5 | Added ClassVar typing and parent-child validation rules to enforce proper Dialog component hierarchy. Missing _valid_children for DialogPortal. |
| pyi_hashes.json | 5/5 | Updated hash for dialog.pyi stub file to reflect changes in dialog.py. |
Sequence Diagram
sequenceDiagram
participant User
participant DialogRoot
participant DialogTrigger
participant DialogPortal
participant DialogOverlay
participant DialogContent
User->>DialogRoot: Create Dialog component
DialogRoot->>DialogTrigger: Add trigger as child
DialogRoot->>DialogPortal: Add portal as child
Note over DialogRoot,DialogPortal: _valid_children enforcement
DialogPortal->>DialogOverlay: Render overlay
DialogPortal->>DialogContent: Render content
Note over DialogPortal,DialogContent: _valid_parents enforcement
User->>DialogTrigger: Click trigger
DialogTrigger->>DialogRoot: Trigger open state
DialogRoot->>DialogPortal: Update open state
DialogPortal->>DialogOverlay: Show overlay
DialogPortal->>DialogContent: Show content
Additional Comments (1)
-
reflex/components/radix/primitives/dialog.py, line 44-57 (link)logic:
DialogPortalis missing_valid_childrendefinition. According to Radix UI Dialog structure, it should containDialogOverlayandDialogContent.
2 files reviewed, 1 comment
masenf
approved these changes
Nov 4, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.