-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Setting translate through direct access does not work #3800
Conversation
📊 Tachometer Benchmark ResultsSummaryduration
usedJSHeapSize
Results02_replace1k
duration
usedJSHeapSize
run-warmup-0
run-warmup-1
run-warmup-2
run-warmup-3
run-warmup-4
run-final
02_replace1k
duration
usedJSHeapSize
run-warmup-0
run-warmup-1
run-warmup-2
run-warmup-3
run-warmup-4
run-final
03_update10th1k_x16
duration
usedJSHeapSize
03_update10th1k_x16
duration
usedJSHeapSize
07_create10k
duration
usedJSHeapSize
07_create10k
duration
usedJSHeapSize
filter_list
duration
usedJSHeapSize
filter_list
duration
usedJSHeapSize
hydrate1k
duration
usedJSHeapSize
hydrate1k
duration
usedJSHeapSize
many_updates
duration
usedJSHeapSize
many_updates
duration
usedJSHeapSize
text_update
duration
usedJSHeapSize
text_update
duration
usedJSHeapSize
todo
duration
usedJSHeapSize
todo
duration
usedJSHeapSize
|
Size Change: +39 B (0%) Total Size: 60.1 kB
ℹ️ View Unchanged
|
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.
Good catch! Let's add a test for that 👍
I feel like this might be better suited to compat. The correct usage would be |
@developit it's a valid attribute value so I would say this belongs in core. The attribute is correctly treated with EDIT: will create a test later, was a phone PR |
@developit both "yes" and "no" are valid values according to mdn: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/translate . |
@marvinhagemeister those values are for the attribute, not the property. |
Closing due to inactivity |
Hi! Is there an update on whether this will be merged and released? |
@JoviDeCroock 👋 Bumping my comment above about whether this PR will be merged in at any point? Our team had to do a manual patch of preact using |
Fixes #3799
This changes our JSX type to reflect the fact that we support Boolean as a value for the property not yes and no.
However in compat we reflect Reacts behavior of supporting yes | no
For the record, React does not support booleans here https://stackblitz.com/edit/stackblitz-starters-tp34wv?description=React%20%20%20TypeScript%20starter%20project&file=src%2FApp.tsx&title=React%20Starter