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

Fixes checked property being reset on render #2907

Merged
merged 1 commit into from Oct 8, 2022

Conversation

WorldSEnder
Copy link
Member

@WorldSEnder WorldSEnder commented Oct 4, 2022

If the value is uncontrolled, do not touch it. Only if it is explicitly given (controlled) set it with every render.

A boolean attribute can have two states: it can exist or be omitted (true and false).
A boolean property can have three states in vdom: it can be explicitly assigned true, explicitly assigned false or it can be left uncontrolled. The last has been missing and the buggy behaviour assumes the last is synonymous with "explicitly false".

Marked as a breaking change, because the API exposed by vdom changes slightly. This is not technically necessary for the PR, but it should serve as a reminder for anybody using the method that there was something subtly wrong with it, as it also assumed a default-unchecked which is not true.

- pub fn VTag::checked(&self) -> bool;
+ pub fn VTag::checked(&self) -> Option<bool>;
+ pub fn VTag::preserve_checked(&mut self);

Description

Fixes #2875. I will add tests when I have time in the coming days.

Checklist

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

if the value is uncontrolled, do not touch it. Only if it is explicitly given
@WorldSEnder WorldSEnder added A-yew Area: The main yew crate A-yew-macro Area: The yew-macro crate labels Oct 4, 2022
@github-actions
Copy link

github-actions bot commented Oct 4, 2022

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

https://yew-rs-api--pr2907-fix-checkbox-3r5hjhns.web.app

(expires Tue, 11 Oct 2022 09:29:12 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@github-actions
Copy link

github-actions bot commented Oct 4, 2022

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 345.847 347.271 346.320 0.410
Hello World 10 622.304 626.181 623.166 1.155
Function Router 10 2192.283 2208.406 2199.579 4.814
Concurrent Task 10 1007.990 1009.628 1008.805 0.578

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 307.364 312.694 308.912 2.011
Hello World 10 603.014 605.245 603.820 0.614
Function Router 10 2190.796 2246.407 2214.240 14.082
Concurrent Task 10 1008.384 1009.677 1008.899 0.383

@github-actions
Copy link

github-actions bot commented Oct 4, 2022

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
async_clock 106.217 106.233 +0.017 +0.016%
boids 170.378 170.401 +0.023 +0.014%
communication_child_to_parent 90.110 90.125 +0.015 +0.016%
communication_grandchild_with_grandparent 104.910 104.962 +0.052 +0.049%
communication_grandparent_to_grandchild 100.767 100.826 +0.060 +0.059%
communication_parent_to_child 87.214 87.232 +0.019 +0.021%
contexts 107.382 107.444 +0.062 +0.058%
counter 85.128 85.148 +0.021 +0.024%
counter_functional 85.601 85.619 +0.019 +0.022%
dyn_create_destroy_apps 87.963 87.982 +0.020 +0.022%
file_upload 99.700 99.717 +0.017 +0.017%
function_memory_game 162.823 162.849 +0.025 +0.016%
function_router 347.237 347.283 +0.046 +0.013%
function_todomvc 157.660 157.688 +0.028 +0.018%
futures 222.020 222.036 +0.017 +0.007%
game_of_life 104.622 104.644 +0.021 +0.021%
immutable 181.201 181.219 +0.018 +0.010%
inner_html 81.991 82.011 +0.020 +0.024%
js_callback 110.913 110.976 +0.062 +0.056%
keyed_list 195.485 195.503 +0.018 +0.009%
mount_point 84.739 84.758 +0.019 +0.022%
nested_list 111.664 111.725 +0.061 +0.054%
node_refs 92.611 92.632 +0.021 +0.022%
password_strength 1546.993 1547.012 +0.019 +0.001%
portals 95.977 96.043 +0.066 +0.069%
router 317.025 317.094 +0.068 +0.022%
simple_ssr 151.692 151.759 +0.066 +0.044%
ssr_router 392.376 392.445 +0.069 +0.018%
suspense 108.660 108.722 +0.062 +0.057%
timer 87.993 88.013 +0.020 +0.022%
todomvc 138.957 138.977 +0.020 +0.014%
two_apps 85.794 85.813 +0.020 +0.023%
web_worker_fib 152.043 152.066 +0.023 +0.015%
webgl 84.434 84.450 +0.017 +0.020%

✅ 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.

Looks good to me

@hamza1311 hamza1311 merged commit bb71b5d into yewstack:master Oct 8, 2022
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 A-yew-macro Area: The yew-macro crate breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Checkbox will lose the checked value after the component get updated.
2 participants