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

Added node! convenience macro #265

Merged
merged 3 commits into from
Sep 30, 2021
Merged

Conversation

jquesada2016
Copy link
Contributor

This macro works well for cases where we need access to underlying GenericNode operations, such as .event(), .add_attribute(), .remove_attribute that cannot currently be performed inside of the template! macro, yet retain the ergonomics for setting attributes and children that don't need the extra flexibility. Please see the doc example below for a possible use case.

This is also a good workaround for #264.

Copy link
Member

@lukechu10 lukechu10 left a comment

Choose a reason for hiding this comment

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

lgtm. Just two nitpicks.

@jquesada2016
Copy link
Contributor Author

Applied suggested changes!

@lukechu10
Copy link
Member

The diff has some commits that are not supposed to be there. If you don't mind, I can fix this and I'll get this merged

@jquesada2016
Copy link
Contributor Author

I removed all commits that weren't supposed to be there. It was the 0.6.1 release that I pulled in. If there's still something that isn't supposed to be there, go right ahead!

@jquesada2016 jquesada2016 mentioned this pull request Sep 26, 2021
@lukechu10
Copy link
Member

Sorry for the run-around but it seems like my suggested changes got undone.

@jquesada2016
Copy link
Contributor Author

No worries! Let me know what you would like me to do in this case.

@lukechu10 lukechu10 merged commit 99a3ff0 into sycamore-rs:master Sep 30, 2021
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.

2 participants