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

Refactor Child Rendering to accept Observable #4

Merged
merged 12 commits into from
Jul 21, 2024
Merged

Conversation

hmallen99
Copy link
Owner

@hmallen99 hmallen99 commented Jul 21, 2024

The createElement function currently accepts a list of child nodes that are immediately rendered, which makes it hard for content to be dynamic. To handle one part of this, the Ternary component was implemented, which accepted a boolean observable and returned a truthy or falsy node. This does not feel like the correct API, though, as we would need to special case switch statements, if then statements, and the null case, not to mention rendering lists of content.

To solve this issue, we should instead accept an observable as the children argument in createElement. This observable emits a stable key and a PipeNode, The latter of which gets attached to the parent element (or updates an existing element with the stable key). This allows for all of the dynamic content items to be implemented at once, rather than special casing each of them.

Copy link

github-actions bot commented Jul 21, 2024

LCOV of commit 226b733 during Continuous Integration Checks #15

Summary coverage rate:
  lines......: 100.0% (122 of 122 lines)
  functions..: 100.0% (9 of 9 functions)
  branches...: 92.6% (25 of 27 branches)

Files changed coverage rate:
                                    |Lines       |Functions  |Branches    
  Filename                          |Rate     Num|Rate    Num|Rate     Num
  ========================================================================
  packages/pipe/src/createElement.ts| 100%    102| 100%     6| 100%     22

@hmallen99 hmallen99 merged commit 33f732b into main Jul 21, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant