Skip to content

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented Sep 8, 2019

We should probably finish #2465, #2462 and look into #2523 before this.

Fixes #2542

Preview: https://elastic-wing-9ce26c.netlify.com/en/

@XhmikosR
Copy link
Contributor Author

How about we move all JS (maybe not the IE custom modernizr load) to a file named main.js instead of 2 files? I know we have http/2, but these are small files and this way we keep things simple.

@XhmikosR
Copy link
Contributor Author

I will go with the one file solution. This way we won't have multiple JS files and we won't need to look into concatenating them on build.

I think it's a good compromise given the nature of the project.

@XhmikosR XhmikosR marked this pull request as ready for review September 30, 2019 19:26
@XhmikosR
Copy link
Contributor Author

Marked it as ready for review.

I only left the IE modernizer check in the HTML. Let me know what you guys think. (Preview #2543 (comment))

PS. I hope no one links to download.js from other Node.js sites. If so, we'll need to rethink this.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 1, 2019

I moved everything to main.js. Now the HTML files are free from inline JS.

@Trott
Copy link
Member

Trott commented Oct 1, 2019

@nodejs/website This could use some reviews.

@Trott Trott merged commit 2a111ce into nodejs:master Oct 1, 2019
@XhmikosR XhmikosR deleted the master-xmr-mv-js branch October 1, 2019 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move inline JS outside of HTML
4 participants