Removing IE9 and IE10 support #368
Conversation
diegonvs
changed the title
|
Great, @diegonvs! We also need to go through code removals like https://github.com/metal/metal.js/blob/master/packages/metal-dom/src/domNamed.js#L413-L453 If it is supported in IE11 and the rest of our support matrix, then we can get rid of some of those fallbacks (in case there are more) |
diegonvs
force-pushed
the
remove-ie9-10-support
branch
2 times, most recently
from
cf0ab19 to
bf397ee
Compare
packages/metal-dom/src/domNamed.js
Outdated
| * @return {boolean} | ||
| */ | ||
| export function contains(element1, element2) { | ||
| if (isDocument(element1)) { |
There was a problem hiding this comment.
Hey @diegonvs, this is actually a breaking change we shouldn't do. It offers a different API (not that it is super helpful...).
Just remove the if clause and keep only the return element1.contains(element2) code.
We could also deprecate it as well, so we get rid of it in a next major release, adding the message:
@deprecated Use element1.contains(element2) directly instead of this method
There was a problem hiding this comment.
packages/metal-dom/src/domNamed.js
Outdated
| @@ -854,7 +839,7 @@ function toggleClassesWithoutNative_(element, classes) { | |||
| */ | |||
| function triggerElementListeners_(element, event, defaultFns) { | |||
| const lastContainer = event[LAST_CONTAINER]; | |||
| if (!isDef(lastContainer) || !contains(lastContainer, element)) { | |||
| if (!isDef(lastContainer) || !lastContainer.contains(element)) { | |||
There was a problem hiding this comment.
This should be fine, as I said above, just keep the method and deprecate it, and remove all of our existing usages like this 👍
packages/metal-dom/src/features.js
Outdated
| @@ -56,22 +55,6 @@ class features { | |||
| } | |||
| return `${type}end`; | |||
| } | |||
|
|
|||
| /** | |||
| * Some browsers (like IE9) change the order of element attributes, when html | |||
There was a problem hiding this comment.
The comment says IE9, but we should check in the real browsers to see which ones actually do this.
diegonvs
force-pushed
the
remove-ie9-10-support
branch
from
bf397ee to
2ce01ff
Compare
diegonvs
force-pushed
the
remove-ie9-10-support
branch
2 times, most recently
from
fa6bfd0 to
6970b85
Compare
diegonvs
force-pushed
the
remove-ie9-10-support
branch
from
6970b85 to
5b729e7
Compare
diegonvs
changed the title
packages/metal-dom/src/domNamed.js
Outdated
| @@ -209,15 +209,11 @@ export function buildFragment(htmlString) { | |||
| * Checks if the first element contains the second one. | |||
| * @param {!Element} element1 | |||
| * @param {!Element} element2 | |||
| * @deprecated Use element1.contains(element2) directly instead of this method | |||
There was a problem hiding this comment.
Can you move this on top of the other @param lines? 🙏
|
Hey @diegonvs, I just added a small comment and also, I think we should keep the Would this be all of the polyfills/special code for IE9-IE10 we can get rid of? |
diegonvs
force-pushed
the
remove-ie9-10-support
branch
from
3ebad15 to
e10fe28
Compare
|
Hey @jbalsas, i've undo deletion of polyfills for async right now. These changes I made were the only ones I thought we could remove without compromising support for IE11, probably with the withdrawal of IE11 support in the future we will make big changes and remove many polyfills. |
|
@jbalsas, I can add in each JSDOC an annotation that represents that this function is being used as polyfill for IE11, what do you think? |
No description provided.