-
-
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 context & job agent #2295
Conversation
Visit the preview URL for this PR (updated for commit 06b2b55): https://yew-rs--pr2295-remove-context-job-a-mgv273ou.web.app (expires Sun, 02 Jan 2022 14:13:38 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
We should rename the multi_thread example to either agents or workers. Since all major browsers support WASM Threads, I feel like the title of the example is misleading, considering it doesn't use threads. Pub sub example can also be renamed to contexts (or similar) to better describe what it represents. Also, this seems like a good opportunity to use the new syntax introduced in #2292 in the examples which recently switched to function components. |
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.
Looks good. Just need one change before it can merged.
Co-authored-by: Muhammad Hamza <[email protected]>
Cargo.toml
Outdated
"examples/agents", | ||
"examples/nested_list", | ||
"examples/node_refs", | ||
"examples/password_strength", | ||
"examples/portals", | ||
"examples/pub_sub", | ||
"examples/contexts", | ||
"examples/router", |
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.
Please sort this alphabetically so it's easier to find members based on the directory structure
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.
left minor not blocking comments
use msg_ctx::MessageProvider; | ||
|
||
#[function_component] | ||
pub fn Model() -> Html { |
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 would name this App
and keep it in a separate app.rs
file, but up to you m8 :D
#[function_component(ShowStorageChanged)] | ||
pub fn show_storage_changed() -> Html { |
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.
#[function_component(ShowStorageChanged)] | |
pub fn show_storage_changed() -> Html { | |
#[function_component] | |
pub fn ShowStorageChanged() -> Html { |
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.
LGTM.
@voidpumpkin, we can do renaming in the future.
Description
Fixes #2219
Closes #1080
This pull request remove Context and Job agent and replace them with examples in Context API.
Checklist
cargo make pr-flow