Skip to content
This repository has been archived by the owner on Mar 20, 2018. It is now read-only.

Website Optimize #217

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

maruilian11
Copy link

No description provided.


<!-- jquery-->
<script src="//ajax.googleapis.com/ajax/libs/jquery/1.11.0/jquery.min.js"></script>
<!--<script src="//ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>-->
<script>window.jQuery || document.write('../js/vendor/jquery-1.9.1.min.js"><\/script>')</script>
<script>window.jQuery || document.write('<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/1.9.1/jquery.min.js"><\/script>')</script>
Copy link
Contributor

Choose a reason for hiding this comment

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

obviously wrong here ...

@lmccart
Copy link
Member

lmccart commented Jan 23, 2016

hi @maruilian11 could you please explain your reasoning for making these change? in general it's best to open an issue for discussion first before making a large structural change.

@PeterDaveHello
Copy link
Contributor

@lmccart cdnjs can speed up the load speed of 3-party libraries, and make it easier to manage the repo, these files now don't need to be downloaded in the repo, and in the future, version upgrading can be easily done by changing the version in the url.

@limzykenneth
Copy link
Member

I agree that leveraging cdnjs is a good idea but as far as I know, ISP in some countries are known to block, intentionally or unintentionally, calls to the CDN. What I do in my project is to leverage the cdn while having a backup copy:

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/2.1.4/jquery.min.js"></script>
<script>
    window.jQuery || document.write('<script src="./vendor/jquery.min.js"><\/script>')
</script>

With that the user's experience is guaranteed which I think is more important than the load speed. The jquery library in the original is already using this strategy.

I agree with @lmccart here about opening an issue first if it is a large structural change, not to micro manage but to discuss before anyone put any effort into changing things which we might change drastically after the discussion here at the PR.

PS. @maruilian11 I've added line note on the latest commit as well

@lmccart
Copy link
Member

lmccart commented Jan 24, 2016

got it, this all makes great sense. i like the idea of providing the backup both in case of cdn fail, and i think eventually it'd be nice to make sure all of this can run and be developed offline for those with limited internet connectivity.

@maruilian11
Copy link
Author

sorry for not opening an issue first, i will delete the last commit to keep jquery file alive in repo.

@limzykenneth
Copy link
Member

@maruilian11 To keep the local copies, for modernizr the test will simply become

window.Modernizr || document.write(LOAD LOCAL COPY);

However, for normalize it is much harder if not impossible. I went digging and found a couple hacks, the first one tests the document.stylesheets.cssRule object which will not work on most browsers now as that violates cross domain policy. The second is to look for existence of specific rule in the stylesheet in the body but that is more catering to bootstrap where there are specific rules set but not normalize which is bunch of fixes to the default css, and it is much messier to implement.

Some also suggest a library like fallback.js but personally I'd say let's not add to the complexity unnecessarily and load normalize from the local copy.

Also, I'm not sure if the files other than header.php needs to be change as I would thought they are dynamically generated with header.php somehow @lmccart ?

@lmccart
Copy link
Member

lmccart commented Jan 25, 2016

The reference pages are automatically generated based on this file, so rather than changing the reference files here, we'll want to update the handlebars layout in the p5.js repo:
https://github.com/processing/p5.js/blob/master/docs/yuidoc-p5-theme/layouts/main.handlebars#L133

I think that should cover it..

@lmccart
Copy link
Member

lmccart commented Feb 26, 2016

hi @maruilian11 what's the status here? have the changes mentioned in the comments been implemented?

@maruilian11
Copy link
Author

so, can we use modernizr.js from cdnjs as a backup copy and keep normalize.css files alive in repo?
is that alright? sorry, my English isn't very good...

about prism, we only have v0.0.1 on cdnjs, but we are trying to add later version, can we wait for the later version being added, then make changes in commit?

@PeterDaveHello
Copy link
Contributor

so, we can use modernizr.js from cdnjs as a backup copy, and keep normalize.css files alive in repo.
is that alright? sorry, my English isn't very good...

Need a detection to fallback

about prism, we only have v0.0.1 on cdnjs, but we are trying to add later version, can we wait for the later version being added, then make changes in commit?

So let's add it right now.

@limzykenneth
Copy link
Member

@maruilian11 we can use modernizr directly from cdnjs while have a local copy as back up:

<script src="https://cdnjs.cloudflare.com/ajax/libs/modernizr/2.8.3/modernizr.min.js"></script>
<script>
    window.modernizr || document.write('<script src="./js/vendor/modernizr.min.js"><\/script>')
</script>

The same goes for prism.js as soon as it's done on cdnjs.

While for all css files, yes we'll keep them as they are. I have some new ideas but I'll try it on my own stuff first before suggesting them here.

If you need any help with this give me a shout any time. : )

@maruilian11
Copy link
Author

@limzykenneth Thank you very much! That’s very kind of you.

@digitalfrost
Copy link

Can be closed as P5.js website has been rebuilt using Node.js, Grunt, YAML and Assemble
See https://github.com/processing/p5.js-website

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

Successfully merging this pull request may close these issues.

5 participants