-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Remove start_app_as_body. #2346
Conversation
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.
Could you add a note about this change in migration docs?
Head branch was pushed to by a user without write access
Co-authored-by: Julius Lungys <[email protected]>
Visit the preview URL for this PR (updated for commit a28a590): https://yew-rs--pr2346-remove-as-body-h887e32t.web.app (expires Sun, 16 Jan 2022 11:09:45 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
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.
Well looks good for me but I would like for @siku2 to take a look as well just in case.
@@ -0,0 +1,8 @@ | |||
--- |
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.
I realized this page is not added to the navigation bar. Could you add it?
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.
I think the migration guide should remain "hidden" until released, shouldn't it?
(This is the same as the yew-agent 0.1.0 -> 0.2.0 guide, which is hidden.)
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.
Showing it would make 0.19 -> next transition easier but it's something we can do later.
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.
you guys are right
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.
Been long enough, if anything we can just implement this again in the future
Hey, Is there any alternative to this method? i want to use |
Yes, what is the canonical way of doing this now? I need to set an attribute on body for css theme reasons and can't figure out how to do it |
You can use If you wish to have body classes managed like If you wish to manage global styling, you can also use |
Description
Fixes #2344
This Pull Request Removes
_as_body
variant ofstart_app
Due to the following reasons:Checklist
cargo make pr-flow