-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Polyfill the window.customElements #23592
Conversation
Co-authored-by: delvh <dev.lh@web.de>
I don't think it's worth it. Any browser released in the last 5 years should have it. If such an ancient browser is used, I assume more stuff will be broken. Also, the place where you polyfill is incorrect, it should be a separate file so that if more components are added, you avoid the double import. |
But we ever spent more time on older browsers, like this one:
Hmm ... same reporter in comment .... About "Also, the place where you polyfill is incorrect, it should be a separate file so that if more components are added, you avoid the double import." I know. But, it's not worth to make it that complex (there is only one file at the moment). Next time, when adding more components, there will be definitely a separate |
PaleMoon only got it very recently, so yeah probably needed. Can you just move the polyfill to |
I am not sure whether I could do that as your expectation, could you help to make that change? You have the writer access to my fork. |
Actually a |
I'm not sure if that polyfill is going to work because it seems it does not support extending. There is another one around that does. |
I have checked, and we do not need Since |
I think we need the other polyfill because we do |
Checked, it really works. The fact is: our component never depends on the builtin element's behavior, so we are not extending them. See my screenshot: the right one is with the polyfill (I also added a temp component on the page to show it works). |
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.
I guess it's okay as short-term fix. When we add another web component, we need to move the polyfill.
I was unable to create a backport for 1.19, please send one manually. 🍵 |
* upstream/main: Replace a few fontawesome icons with svg (go-gitea#23602) Fix pagination on `/notifications/watching` (go-gitea#23564) Fix `.locale.Tr` function not found in delete modal (go-gitea#23468) fix submodule is nil panic (go-gitea#23588) `Publish Review` buttons should indicate why they are disabled (go-gitea#23598) Improve template error reporting (go-gitea#23396) Polyfill the window.customElements (go-gitea#23592) Add CHANGELOG for 1.19.0 (go-gitea#23583)
Related: #23590
Reference: https://github.com/webcomponents/polyfills/tree/master/packages/webcomponentsjs
It seems that some browsers don't support customElements.
The Custom Elements would help a lot for Gitea's UI problems, including:
<span class="js-pretty-number">
<time data-format>
So it's worth get polyfill.