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.

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

Use YUIDoc theme helpers for local development. #1124

Merged
merged 1 commit into from Nov 24, 2015

Conversation

toolness
Copy link
Member

It seems like 4de4a31 removed a bunch of duplicate files that are on p5.js-website, which makes the project nice and DRY, but this had the downside of making the generated yuidocs in /docs/reference/ less useful, as the CSS is broken and example sketches don't render.

There's a few potential fixes to this that I can think of:

  1. Add a step to grunt yui that downloads the required p5.js-website assets to the directories where the yuidoc theme expects them to be.
  2. Document a process by which devs can manually check out p5.js-website on their own and copy/symlink the directories themselves.
  3. Change the yuidoc theme so it can link directly to assets on p5js.org and the p5 libraries on the local dev server, with an option to link to assets the "old way" if the P5_SITE_ROOT environment variable is set to ...

This implements option (3), though I'm not sure how much I like it. For instance:

  • Due to CORS issues, the fonts can't be loaded, so things still look a little bit broken. (Though this can be fixed if p5js.org adds CORS headers, that might not be preferable.)
  • Because the theme links directly to p5js.org, the docs can't be browsed offline.
  • It also adds a bit of complexity to the build process that may be a hassle to maintain in the future.

On the plus side, though, the docs now link directly to the fresh p5 libraries that were just built by grunt, which makes the docs and examples easier to test.

All that said, I'm fine with this PR being rejected if one of the alternatives is a better fit.

lmccart pushed a commit that referenced this pull request Nov 24, 2015
Use YUIDoc theme helpers for local development.
@lmccart lmccart merged commit 48f1b8c into processing:master Nov 24, 2015
@lmccart
Copy link
Member

lmccart commented Nov 24, 2015

I think this is helpful, thanks! I suppose we could add CORS headers to p5js.org, what are the potential downsides of that?

@toolness
Copy link
Member Author

Cool, no problem!

Adding CORS headers to just the css/f directory on p5js.org has no downsides that I can think of--basically, according to a Mozilla developer, font foundries were worried about font piracy so the CORS requirement was just added to the spec to allay their fears.

Are Avenir Next and Inconsolata under a free license? If they are, I doubt there's any downside to CORS'ing the font directory.

@lmccart
Copy link
Member

lmccart commented Nov 25, 2015

ok great, I added CORS headers so the fonts display correctly. there's still a tiny bug with the spacing in the header, but I think it's close enough that this can be used easily now to confirm the reference looks right.

@toolness
Copy link
Member Author

Awesome, thanks!

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.

None yet

2 participants