-
Notifications
You must be signed in to change notification settings - Fork 568
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
Support conditional children in most JSX components #2506
Conversation
@metamaskbot update-pr |
@@ -161,7 +161,7 @@ export const OptionStruct: Describe<OptionElement> = element('Option', { | |||
export const DropdownStruct: Describe<DropdownElement> = element('Dropdown', { | |||
name: string(), | |||
value: optional(string()), | |||
children: maybeArray(OptionStruct), | |||
children: maybeArray(nullUnion([OptionStruct, boolean()])), |
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.
Realizing that this means that null isn't allowed in Dropdowns currently.
Would it be easier to have a jsxChildren
struct that takes a list of structs and combines that with boolean()
, null
etc?
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.
Sure, that does sound easier.
@@ -6,7 +6,7 @@ import type { StandardFormattingElement } from './formatting'; | |||
* The children of the {@link Link} component. | |||
*/ | |||
export type LinkChildren = MaybeArray< |
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.
Same question with regards to types, would it be easier to have a shared type that adds boolean | null
and whatever else
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2506 +/- ##
=======================================
Coverage 94.40% 94.40%
=======================================
Files 442 442
Lines 9125 9127 +2
Branches 1411 1412 +1
=======================================
+ Hits 8614 8616 +2
Misses 511 511 ☔ View full report in Codecov by Sentry. |
ebed3d9
to
4fff8c8
Compare
This adds support for conditional children in most JSX components. The exceptions are
Field
,Row
, and other components which don't have children in the first place. For example, the following is now possible:To support this, I've added
boolean
as valid prop for the JSX components. ThegetJsxChildren
function filters out any falsy or literaltrue
values.I've also added a small contributing guide for conventions around JSX components, including the support for conditional children.
Closes #2505.