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.

Sign up for GitHub

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

Jump to bottom

ui: remove experimental API in checkBrowser.js #776

Merged
merged 4 commits into from
Oct 22, 2020
Merged

ui: remove experimental API in checkBrowser.js #776

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8e3e926
ui: remove experimental API in checkBrowser.js
unbyte Oct 22, 2020
0f146cd
ui: remove experimental API in checkBrowser.js
unbyte Oct 22, 2020
5dfd61a
ui: polyfill for Object.entries
unbyte Oct 22, 2020
3b4deac
Merge branch 'master' into fix-check-browser
unbyte Oct 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ui/public/checkBrowser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function checkBrowser() {
d.getElementsByTagName('a')[0].onclick = function () {
d.getElementsByTagName('div')[0].style.top = '-60px'
}
document.body.prepend(d)
document.body.prepend ? document.body.prepend(d) : document.body.insertBefore(d, document.body.firstChild)
Copy link
Collaborator

@baurine baurine Oct 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I prefer to directly use document.body.insertBefore(d, document.body.firstChild) because it can be used in all browsers.

We can use 1 PR to resolve the problems of #770 together (will discuss it later).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, however, the support for firstChild in chromes <=85 is unknown as caniuse shows

Copy link
Contributor

@kennytm kennytm Oct 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

how about childNodes[0].

but i highly doubt Chrome 85- not supporting firstChild which is in DOM 1 released back in 1998.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just curious about why support is unknown. maybe there is a special reason?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is fine, according to this doc: https://developer.mozilla.org/zh-CN/docs/Web/API/ParentNode/prepend , prepend polyfill is implemented by insertBefore and firstChild, even if document.body.firstChild is undefined, it works as expected, or we can consider using appendChild?

Copy link
Contributor Author

@unbyte unbyte Oct 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

unknown comes from no real data yet. see mdn/browser-compat-data#6369
so yeah we can use insertBefore directly

}
}

Expand Down