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 webkitdirectory to the boolean attributes list #3214

Merged

Conversation

voidpumpkin
Copy link
Member

@voidpumpkin voidpumpkin commented Apr 3, 2023

Description

Fixes #2968

Checklist

  • I have reviewed my own code
  • I have added tests
  • I tested manually as this a bit of a custom case

@voidpumpkin voidpumpkin added the A-yew Area: The main yew crate label Apr 3, 2023
github-actions[bot]
github-actions bot previously approved these changes Apr 3, 2023
@github-actions
Copy link

github-actions bot commented Apr 3, 2023

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

https://yew-rs-api--pr3214-bool-into-prop-value-gcv6mn98.web.app

(expires Tue, 11 Apr 2023 19:54:33 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@github-actions
Copy link

github-actions bot commented Apr 3, 2023

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
async_clock 104.221 104.221 0 0.000%
boids 174.482 174.482 0 0.000%
communication_child_to_parent 96.494 96.494 0 0.000%
communication_grandchild_with_grandparent 108.454 108.454 0 0.000%
communication_grandparent_to_grandchild 104.707 104.707 0 0.000%
communication_parent_to_child 93.553 93.553 0 0.000%
contexts 109.524 109.524 0 0.000%
counter 90.289 90.289 0 0.000%
counter_functional 90.622 90.622 0 0.000%
dyn_create_destroy_apps 93.141 93.141 0 0.000%
file_upload 104.141 104.141 0 0.000%
function_memory_game 168.537 168.537 0 0.000%
function_router 337.636 337.636 0 0.000%
function_todomvc 162.881 162.881 0 0.000%
futures 227.546 227.546 0 0.000%
game_of_life 110.459 110.459 0 0.000%
immutable 188.956 188.956 0 0.000%
inner_html 86.941 86.941 0 0.000%
js_callback 113.798 113.798 0 0.000%
keyed_list 201.070 201.070 0 0.000%
mount_point 90.053 90.053 0 0.000%
nested_list 114.402 114.402 0 0.000%
node_refs 97.290 97.290 0 0.000%
password_strength 1544.746 1544.746 0 0.000%
portals 98.554 98.554 0 0.000%
router 308.209 308.209 0 0.000%
simple_ssr 144.174 144.174 0 0.000%
ssr_router 374.432 374.432 0 0.000%
suspense 110.974 110.974 0 0.000%
timer 93.186 93.186 0 0.000%
todomvc 144.483 144.483 0 0.000%
two_apps 90.943 90.943 0 0.000%
web_worker_fib 154.883 154.883 0 0.000%
webgl 89.580 89.580 0 0.000%

✅ None of the examples has changed their size significantly.

@github-actions
Copy link

github-actions bot commented Apr 3, 2023

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 345.514 381.244 366.272 11.560
Hello World 10 689.302 738.213 714.543 17.541
Function Router 10 2521.955 2737.199 2582.716 67.492
Concurrent Task 10 1009.169 1010.602 1009.675 0.524
Many Providers 10 1834.711 1966.913 1885.465 42.908

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 342.073 382.073 357.650 12.468
Hello World 10 691.173 731.334 708.978 15.815
Function Router 10 2455.343 2581.827 2529.991 33.827
Concurrent Task 10 1007.754 1010.138 1009.123 0.790
Many Providers 10 1764.078 1885.489 1828.771 41.436

@futursolo
Copy link
Member

futursolo commented Apr 4, 2023

I think we need to set / unset this attribute if its value is set to boolean or delegate this attribute to property.

Boolean attributes dictates whether it is truthy or falsy by its presence.
That is disabled={false} will cause an element to be disabled.

See: https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes#boolean_attributes

In addition, implicit string conversion feels like too JavaScript-ish to me. This is usually discouraged in Rust.

@voidpumpkin voidpumpkin added the S-waiting-on-author Status: awaiting action from the author of the issue/PR label Apr 4, 2023
@voidpumpkin voidpumpkin changed the title Implement bool IntoPropValue String & AttrValue Add webkitdirectory to the boolean attributes list Apr 4, 2023
@voidpumpkin
Copy link
Member Author

voidpumpkin commented Apr 4, 2023

If I understood correctly webkitdirectory is an attribute with wide coverage in browsers, but has not been fully standardized. This is why it ia not present in the list.

The spec also mentions that webkitdirectory is not yet standardized, ctlf+f webkitdirectory in the spec to see the note.

@voidpumpkin voidpumpkin removed the S-waiting-on-author Status: awaiting action from the author of the issue/PR label Apr 4, 2023
@voidpumpkin voidpumpkin merged commit 24d79e8 into yewstack:master Apr 5, 2023
@voidpumpkin voidpumpkin deleted the bool_into_prop_value_string branch April 5, 2023 18:40
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.

webkitdirectory on input tag fails
2 participants