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

Add method map() on Children to wrap easily #3039

Merged
merged 2 commits into from Jan 5, 2023

Conversation

cecton
Copy link
Member

@cecton cecton commented Dec 13, 2022

Description

Add a function Children::map(&self) to easily wrap children.

Real life example:

use yew::prelude::*;

#[derive(Clone, PartialEq, Properties)]
pub struct ButtonProps {
    #[prop_or_default]
    pub onclick: Callback<MouseEvent>,
    #[prop_or_default]
    pub children: html::Children,
}

#[function_component(Button)]
pub fn button(props: &ButtonProps) -> Html {
    let ButtonProps {
        onclick,
        children,
        // ...
    } = props;

    html! {
        <button
            // ...
            onclick={(!disabled).then_some(onclick.clone())}
        >
            {
                (!children.is_empty())
                    .then(|| html! {
                        <span class="bp3-button-text">
                            {for children.iter()}
                        </span>
                    })
            }
        </button>
    }
}

This can be simplified into:

{
    (children
        .map(|children| html! {
            <span class="bp3-button-text">
                {for children.iter()}
            </span>
        })
}

Checklist

  • I have reviewed my own code
  • I have added tests

github-actions[bot]
github-actions bot previously approved these changes Dec 13, 2022
@github-actions
Copy link

github-actions bot commented Dec 13, 2022

Visit the preview URL for this PR (updated for commit afcce29):

https://yew-rs-api--pr3039-add-convient-helper-dsruq0op.web.app

(expires Tue, 20 Dec 2022 15:39:28 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@github-actions
Copy link

github-actions bot commented Dec 13, 2022

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 376.771 391.559 379.649 4.796
Hello World 10 681.386 690.918 683.508 3.026
Function Router 10 2366.675 2383.676 2374.848 5.059
Concurrent Task 10 1007.733 1009.042 1008.461 0.395

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 376.776 391.476 381.978 5.052
Hello World 10 734.835 744.337 737.709 3.512
Function Router 10 2405.180 2464.383 2450.333 17.120
Concurrent Task 10 1007.151 1009.236 1008.228 0.689

@github-actions
Copy link

github-actions bot commented Dec 13, 2022

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
async_clock 108.258 108.259 +0.001 +0.001%
boids 172.289 172.295 +0.006 +0.003%
communication_child_to_parent 92.363 92.360 -0.003 -0.003%
communication_grandchild_with_grandparent 106.797 106.799 +0.002 +0.002%
communication_grandparent_to_grandchild 102.658 102.652 -0.006 -0.006%
communication_parent_to_child 89.445 89.444 -0.001 -0.001%
contexts 109.335 109.336 +0.001 +0.001%
counter 87.386 87.385 -0.001 -0.001%
counter_functional 87.802 87.798 -0.004 -0.004%
dyn_create_destroy_apps 90.233 90.234 +0.001 +0.001%
file_upload 101.708 101.706 -0.002 -0.002%
function_memory_game 166.475 166.475 0 0.000%
function_router 350.031 350.035 +0.004 +0.001%
function_todomvc 161.419 161.418 -0.001 -0.001%
futures 226.534 226.535 +0.001 +0.000%
game_of_life 108.142 108.142 0 0.000%
immutable 183.999 183.999 0 0.000%
inner_html 83.808 83.807 -0.001 -0.001%
js_callback 113.015 113.009 -0.006 -0.005%
keyed_list 197.806 197.808 +0.002 +0.001%
mount_point 86.957 86.956 -0.001 -0.001%
nested_list 114.942 114.942 0 0.000%
node_refs 94.876 94.872 -0.004 -0.004%
password_strength 1552.856 1552.857 +0.001 +0.000%
portals 98.146 98.146 0 0.000%
router 320.127 320.122 -0.005 -0.002%
simple_ssr 153.047 153.056 +0.009 +0.006%
ssr_router 394.512 394.511 -0.001 -0.000%
suspense 110.762 110.762 0 0.000%
timer 90.282 90.279 -0.003 -0.003%
todomvc 142.749 142.748 -0.001 -0.001%
two_apps 88.016 88.014 -0.002 -0.002%
web_worker_fib 154.376 154.375 -0.001 -0.001%
webgl 86.532 86.533 +0.001 +0.001%

✅ None of the examples has changed their size significantly.

Copy link
Member

@hamza1311 hamza1311 left a comment

Choose a reason for hiding this comment

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

Good addition!

Side note: the example in the PR description is wrong. The closure passed to map should provide children as a parameter

Copy link
Member

@WorldSEnder WorldSEnder left a comment

Choose a reason for hiding this comment

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

Looks good to me too! I'm not 100% on board with the name (cause map on an iterator-like thing might mean per-item), but I can't think of a better name, and it's consistent with Option, so 👍 from me.

@cecton
Copy link
Member Author

cecton commented Jan 5, 2023

@WorldSEnder I have the same doubt to be honest but I couldn't find a better name.

There is Option::and_then. But I think it doesn't sound as good as map here.

The actual map on iterator can still be done with children.iter().map(...) (vs children.map(...)).

(I'm merging this for now but I will create another PR if we come up with a better name)

@cecton cecton merged commit d4c2f03 into yewstack:master Jan 5, 2023
@cecton cecton deleted the add-convient-helper-on-children branch January 5, 2023 10:30
@WorldSEnder WorldSEnder added the A-yew Area: The main yew crate label Jan 9, 2023
WorldSEnder pushed a commit to WorldSEnder/yew that referenced this pull request Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew Area: The main yew crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants