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

onsubmit should be a SubmitEvent #2816

Merged
merged 4 commits into from Aug 7, 2022

Conversation

hamza1311
Copy link
Member

Description

onsubmit should be a SubmitEvent, not a FocusEvent

Fixes #2691

This is a breaking change as it changes the public API for the onsubmit event

Checklist

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

@hamza1311 hamza1311 added bug breaking change A-yew Area: The main yew crate labels Aug 7, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 7, 2022
@github-actions
Copy link

github-actions bot commented Aug 7, 2022

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
boids 172.524 172.527 +0.003 +0.002%
contexts 109.253 109.253 0 0.000%
counter 86.471 86.471 0 0.000%
counter_functional 87.123 87.123 0 0.000%
dyn_create_destroy_apps 89.592 89.593 +0.001 +0.001%
file_upload 102.686 102.682 -0.004 -0.004%
function_memory_game 166.349 166.343 -0.006 -0.004%
function_router 350.241 350.456 +0.215 +0.061%
function_todomvc 161.060 161.049 -0.011 -0.007%
futures 225.309 225.312 +0.004 +0.002%
game_of_life 107.063 107.063 0 0.000%
immutable 185.303 185.186 -0.117 -0.063%
inner_html 83.642 83.640 -0.002 -0.002%
js_callback 112.371 112.383 +0.012 +0.010%
keyed_list 195.151 195.151 0 0.000%
mount_point 86.123 86.124 +0.001 +0.001%
nested_list 115.413 115.413 0 0.000%
node_refs 93.400 93.396 -0.004 -0.004%
password_strength 1545.790 1545.784 -0.006 -0.000%
portals 97.209 97.206 -0.003 -0.003%
router 319.777 319.962 +0.185 +0.058%
simple_ssr 154.131 154.139 +0.008 +0.005%
ssr_router 396.165 396.241 +0.076 +0.019%
suspense 110.106 109.963 -0.144 -0.130%
timer 89.201 89.197 -0.004 -0.004%
todomvc 142.516 142.514 -0.002 -0.001%
two_apps 87.140 87.136 -0.004 -0.004%
web_worker_fib 153.532 153.528 -0.004 -0.003%
webgl 87.384 87.385 +0.001 +0.001%

✅ None of the examples has changed their size significantly.

github-actions[bot]
github-actions bot previously approved these changes Aug 7, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 7, 2022
@github-actions
Copy link

github-actions bot commented Aug 7, 2022

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

https://yew-rs-api--pr2816-fix-onsubmit-event-t-hg4nktsv.web.app

(expires Sun, 14 Aug 2022 18:54:43 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@github-actions
Copy link

github-actions bot commented Aug 7, 2022

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 393.510 396.517 393.969 0.912
Hello World 10 1124.052 1153.307 1145.602 8.477
Function Router 10 3392.049 3418.748 3407.184 9.398
Concurrent Task 10 1007.717 1010.018 1009.045 0.690

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 393.459 394.129 393.735 0.217
Hello World 10 1095.412 1104.097 1099.854 3.512
Function Router 10 3456.233 3461.311 3458.378 1.577
Concurrent Task 10 1007.779 1009.761 1008.758 0.634

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.

Have you had time to double check the other event types? Nothing in particular stood out to me at the time, but the link in the linked issue sounded promising for doing that.

@hamza1311 hamza1311 merged commit d422b53 into yewstack:master Aug 7, 2022
@hamza1311
Copy link
Member Author

I didn't get to checking everything but it's a good idea to check them and make sure they're correct

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 breaking change bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

onsubmit should be a SubmitEvent, not a FocusEvent
2 participants