-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fix IE11 not setting initial select value #1838
Conversation
cf169d2
to
39d2a1f
Compare
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.
WOW! Absolutely mental work! 🎉🎊
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.
Wow, nice one! 💯
@@ -88,16 +88,7 @@ function setProperty(dom, name, value, oldValue, isSvg) { | |||
} | |||
} | |||
else if (name!=='list' && name!=='tagName' && !isSvg && (name in dom)) { | |||
// Setting `select.value` doesn't work in IE11. |
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.
this is the best PR I have ever seen
Oh boy, it took me all day to figure this one out. Basically all browsers will infer the option
value
fromtextContent
when it isn't present. It's probably done for backwards compatibility.At this point
option.value
is set and that will skip our own check later:preact/src/diff/index.js
Line 248 in c2909d0
This works fine in all browsers except IE11. When the option value isn't explicitly set, it leads to subtle bugs like
select.value
not working anymore. In this case it will always reset it to an empty string 🤷♂️@andrewiggins this is basically a continuation from #1708 and it turns out that my fix wasn't correct. This PR gets those bytes back 😉
Here is a simple test case to verify the behavior in IE11:
Removes
-24 B
🎉