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

panner-node: Script doesn't load as the DOM is not ready #120

Merged
merged 2 commits into from
Dec 21, 2023
Merged

panner-node: Script doesn't load as the DOM is not ready #120

merged 2 commits into from
Dec 21, 2023

Conversation

shuizhongyueming
Copy link
Contributor

Description

The main.js should load after the DOM structure to make querySelector work correctly

Motivation

The online demo runs failed

Additional details

None

Related issues and pull requests

None

@shuizhongyueming
Update index.html
43b957b 
The main.js should load after the DOM structure to make querySelector work correctly
@github-actions GitHub Actions
Copy link

It looks like this is your first pull request. 🎉
Thank you for your contribution! One of the project maintainers will triage
and assign the pull request for review. We appreciate your patience. To
safeguard the health of the project, please take a moment to read our
code of conduct.

@teoli2003
Copy link
Contributor

teoli2003 commented Dec 21, 2023 •
edited
Loading

Good catch! Thank you!

Unfortunately, your fix is not the correct one. You used the pre-2013 non-standard hack. Since 2008, there has been the defer attribute for this in the spec. Since 2013 all current major browsers have supported it.

Although browsers tolerate it, adding <script> in the <body> invalidates your markup.

I've updated the PR and tested it in Chrome and Firefox. Let's use the 2023 way of doing this!

@teoli2003 teoli2003 changed the title Update index.html panner-node: Script doesn't load as the DOM is not ready Dec 21, 2023
@teoli2003 teoli2003 merged commit cf2dcf0 into mdn:main Dec 21, 2023
2 checks passed
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 21, 2023 •
edited by teoli2003
Loading

Congratulations on your first merged pull request. 🎉 Thank you for your contribution!

@shuizhongyueming
Copy link
Contributor Author

After taking a deep dive into the defer attribute, I totally get why it's a better way to go.

Thanks for making that change and for giving me a chance to learn something new. 🙂

@shuizhongyueming shuizhongyueming deleted the patch-1 branch December 21, 2023 07:30
@teoli2003
Copy link
Contributor

You are very welcome. There are likely other occurrences of this, so don't hesitate to open more PRs.

(IE was the browser holding this feature for a decade, but now it is a thing of the past)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teoli2003 teoli2003 teoli2003 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@shuizhongyueming @teoli2003