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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove IE11 warning message and related files. Fix #7993 #7996

Merged
merged 1 commit into from Feb 22, 2022

Conversation

gianlucadecola
Copy link
Contributor

@gianlucadecola gianlucadecola commented Feb 15, 2022

Referring to #7993 I've removed the warning message related to IE11 and related js code. There are some comments in js referring to IE11, maybe they also can be removed?

// document.currentScript is not available in IE11. Use a fallback instead.

// IE11 crashes when rendering the new entity in contenteditable if the modal is still open.


Thanks for contributing to Wagtail! 馃帀

Before submitting, please review the contributor guidelines https://docs.wagtail.org/en/latest/contributing/index.html and check the following:

@squash-labs
Copy link

squash-labs bot commented Feb 15, 2022

Manage this branch in Squash

Test this branch here: https://gianlucadecolaremove-ie11-warn-uumir.squash.io

@lb-
Copy link
Member

lb- commented Feb 16, 2022

@gianlucadecola thanks heaps for picking this up, I have marked for needs review and triggered the runners.

@thibaudcolas just confirming we are good to do this for 2.17 - I noticed you left these in as part of #7916

@lb- lb- force-pushed the remove-ie11-warning-message branch from ef1c545 to d8d925d Compare February 22, 2022 07:19
Copy link
Member

@lb- lb- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gianlucadecola great stuff! thanks so much for taking the time to contribute this improvement and investigate other issues.

I have pushed to your remote branch with a small change to the docs and also release notes, I have added your name (Gianluca De Cola) to the contributors list. Please let me know if you want the name listed differently.

@lb-
Copy link
Member

lb- commented Feb 22, 2022

@gianlucadecola as for the other items you found that are IE11 specific workarounds, it would be amazing if you could pull these out as a separate PR.

From what I can see, these are good to be removed for our next release.

...here are some comments in js referring to IE11, maybe they also can be removed?

// document.currentScript is not available in IE11. Use a fallback instead.

// IE11 crashes when rendering the new entity in contenteditable if the modal is still open.

@lb- lb- merged commit aaee9b8 into wagtail:main Feb 22, 2022
@thibaudcolas thibaudcolas added this to the 2.17 milestone Feb 23, 2022
@thibaudcolas
Copy link
Member

馃 thank you both! Yes, I can confirm this is good to go for 2.17. Re the other two IE11 workarounds, they technically apply to all browsers. It鈥檇 be nice to remove them, as long as we test this enough to make sure the comments are indeed accurate that this is only needed for IE11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants