Skip to content
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

Merged
merged 10 commits into from
Jun 25, 2024

Conversation

Mrtenz
Copy link
Member

@Mrtenz Mrtenz commented Jun 20, 2024

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:

<Box direction="horizontal" alignment="space-between">
  {condition && <Text>Hello</Text>}
</Box>

To support this, I've added boolean as valid prop for the JSX components. The getJsxChildren function filters out any falsy or literal true values.

I've also added a small contributing guide for conventions around JSX components, including the support for conditional children.

Closes #2505.

@Mrtenz
Copy link
Member Author

Mrtenz commented Jun 20, 2024

@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()])),
Copy link
Member

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?

Copy link
Member Author

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<
Copy link
Member

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

Copy link

codecov bot commented Jun 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.40%. Comparing base (c281824) to head (e73b63d).

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.
📢 Have feedback on the report? Share it here.

@Mrtenz Mrtenz force-pushed the mrtenz/conditional-children branch from ebed3d9 to 4fff8c8 Compare June 25, 2024 09:31
@Mrtenz Mrtenz marked this pull request as ready for review June 25, 2024 10:21
@Mrtenz Mrtenz requested a review from a team as a code owner June 25, 2024 10:21
@Mrtenz Mrtenz merged commit d738f7c into main Jun 25, 2024
156 checks passed
@Mrtenz Mrtenz deleted the mrtenz/conditional-children branch June 25, 2024 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants