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

Allow components to accept children elements #589

Merged
merged 12 commits into from
Sep 10, 2019

Conversation

jstarry
Copy link
Member

@jstarry jstarry commented Aug 13, 2019

Fixes: #537

Terminology

  • (B) Base component that renders components nested inside each other
  • (P) Parent component that has a children property and can render those children
  • (C) Child component that is nested inside parent and included inside the Parent's children

Todo

  • Add example for nested components
  • Support arbitrary html nested inside component tags
  • Support nested components inside component tags
  • Allow modifying & accessing (C) props when rendering (P)
  • Allow filtering (C) components when rendering (P)
  • Children prop be required or optional
  • Clean up nested component example
  • Fix parser for generic component type
  • Write tests
  • Update documentation and README
  • Investigate passing required properties from (P) -> (C)
  • Allow sending messages from (C) -> (B)

@therustmonk
Copy link
Member

That's very important change! Thank you! 👍

@jstarry jstarry changed the title WIP: Allow components to accept children elements Allow components to accept children elements Aug 25, 2019
@jstarry
Copy link
Member Author

jstarry commented Aug 25, 2019

@hgzimmerman could you please review this when you have time?

Copy link
Member

@hgzimmerman hgzimmerman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have extensive experience in writing (or reading for that matter) macros in Rust, which makes up the majority of the delicate parts of this PR. Given my limited understanding, I didn't see anything suspect.

I thought that the code involving Variants in the nested_list example is really clever and cool, but exceptionally non-obvious to someone who hasn't seen that pattern before. I didn't think you could have different child components before seeing this. When a guide/book is written, some special attention will need to be given towards this feature.

Thanks for implementing this feature!

crates/macro/src/html_tree/html_component.rs Show resolved Hide resolved
tests/macro/html-component-pass.rs Outdated Show resolved Hide resolved
@jstarry
Copy link
Member Author

jstarry commented Aug 26, 2019

@hgzimmerman yes, totally agreed about the complexity with the Variants. I think if this causes confusion it could be abstracted away in a macro at some point but I don't think it will be a common pattern. For now, a page in the book would be great. I think I'll start on that soon (mostly about the ins and outs of the macros)

@gtors
Copy link

gtors commented Aug 28, 2019

It would be great if Children will have is_empty method. It useful in case you need to add additional html elements/css styles in case children exist.

Example:

impl MenuItem {
    fn render_caret(&self) -> Html<Self> {
        if self.props.children.is_empty() {
            html! {}
        } else {
            html! {
                <b class="caret"></b>
            }
        }
    }
}

@jstarry
Copy link
Member Author

jstarry commented Sep 5, 2019

@gtors done!

@jstarry
Copy link
Member Author

jstarry commented Sep 10, 2019

bors r+

bors bot added a commit that referenced this pull request Sep 10, 2019
589: Allow components to accept children elements r=jstarry a=jstarry

Fixes: #537

#### Terminology
- (B) Base component that renders components nested inside each other
- (P) Parent component that has a `children` property and can render those children
- (C) Child component that is nested inside parent and included inside the Parent's `children`

#### Todo
- [x] Add example for nested components
- [x] Support arbitrary html nested inside component tags
- [x] Support nested components inside component tags
- [x] Allow modifying & accessing (C) props when rendering (P)
- [x] Allow filtering (C) components when rendering (P)
- [x] Children prop be required or optional
- [x] Clean up nested component example
- [x] Fix parser for generic component type
- [x] Write tests
- [x] Update documentation and README
- [x] ~~Investigate passing required properties from (P) -> (C)~~
- [x] ~~Allow sending messages from (C) -> (B)~~


Co-authored-by: Justin Starry <[email protected]>
@bors
Copy link
Contributor

bors bot commented Sep 10, 2019

Build succeeded

  • continuous-integration/travis-ci/push

@bors bors bot merged commit 429ab73 into yewstack:master Sep 10, 2019
@jstarry jstarry deleted the component-children branch September 25, 2019 02:35
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.

Components should be able to wrap html elements in the html! macro
4 participants