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

jsdoc output #3910

Merged
merged 12 commits into from Jan 24, 2017
Merged

jsdoc output #3910

merged 12 commits into from Jan 24, 2017

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Jan 4, 2017

Uses the TUI JSDoc template. All configuring of jsdoc was moved to the jsdoc.json file.
There's currently a bug with TUI regarding the tutorials (nhn/tui.jsdoc-template#12) but it should via gh-pages currently because it'll now autorender markdown into html.

Left to do:

  • run jsdoc on publish
  • tweak configuration of jsdoc
  • make sure that tutorials are rendering correctly
  • figure out what to do about TOCs in jsdoc
  • fix jsdoc's README

@brandonocasey
Copy link
Contributor

Do we want to do custom styling for video.js in another PR?

@brandonocasey
Copy link
Contributor

Can we change the logo in the top left? (from toastui to videojs)

@gkatsev
Copy link
Member Author

gkatsev commented Jan 4, 2017

Yes, I believe it's all customizable in some way or another. I'll do it in this PR if it isn't too complicated.

@brandonocasey
Copy link
Contributor

I also noticed that some of the links are broken, I think it would be good to have some sort of automated tests to check for broken links (maybe in another PR as well)

@brandonocasey
Copy link
Contributor

I also noticed that examples seem to be cut off at the bottom? (might be a bug in tui)
http://gkatsev.github.io/video.js/docs/api/tutorial-components.html

@gkatsev
Copy link
Member Author

gkatsev commented Jan 4, 2017

the tutorials are loaded in in-an iframe, so, you should be able to just scroll :), but yeah, the iframe is potentially not sized correctly.

Also, which links do you see broken?

@brandonocasey
Copy link
Contributor

the Guides link at http://gkatsev.github.io/video.js/docs/api/index.html leading to http://gkatsev.github.io/video.js/docs/api/guides. Should probably be /guides? I guess its not up yet though.

@brandonocasey
Copy link
Contributor

I also noticed that the API link at the top doesn't just link to the current page.

@gkatsev
Copy link
Member Author

gkatsev commented Jan 4, 2017

Ah, I think that's because it's just inlining the docs/api.md file as a "main" and the links are still relative to the markdown structure. That's something that we'd want to fix. We potentially could just write a different page to use as the front of the docs site.

@gkatsev
Copy link
Member Author

gkatsev commented Jan 10, 2017

The latest updates depend on these changes: https://github.com/nhnent/tui.jsdoc-template/pulls/gkatsev

@gkatsev
Copy link
Member Author

gkatsev commented Jan 23, 2017

Looks like to get tocs to work, we'd need to update the template to add a custom renderer to marked to add embedded anchor tags for headings, I'm going to call that not a blocker.

The only real update that's still necessary and I think we should do it separately from this, probably, is to update the index.md so we have a better output on the main docs page.

@@ -25,10 +25,12 @@
"start": "grunt dev",
"test": "grunt test",
"docs": "npm run docs:lint && npm run docs:api",
"docs:api": "jsdoc -r src/js -d docs/api -c .jsdoc.json",
"jsdoc": "jsdoc",
Copy link
Contributor

Choose a reason for hiding this comment

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

this will only cause "There are no input files to process.", maybe we should skip this script?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is like the grunt task above, it's so that you can more easily run jsdoc commands on the cli without installing it globally if jsdoc -c .jsdoc.json doesn't suit what you're trying to do.

Copy link
Contributor

Choose a reason for hiding this comment

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

seems outside of the scope of what scripts should do. I think its easy enough to do ./node_modules/.bin/jsdoc

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a pretty common pattern nowadays.

"docs:lint": "remark -- './**/*.md'",
"docs:fix": "remark --output -- './**/*.md'",
"babel": "babel src/js -d es5"
"babel": "babel src/js -d es5",
"prepublish": "not-in-install && npm run docs:api || in-install"
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand this right this script will only build the docs prepublish but not when npm install is run on the project. Is that right? (if so this is awesome, and I had no idea that we could do this)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, that's what this does.

"logo": {
"url": "//videojs.com/img/logo.png",
"height": "30px",
"width": "214px"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there anyway to include videojs.com as a link somewhere on the navigation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm... I can definitely add a link on the footer. Making the logo a link would be awesome but might need an update to the template.

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh didn't notice the footer link maybe add an issue to add a link in for the logo to the website.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add it as a note in 6.0 project.

}
},
"logo": {
"url": "//videojs.com/img/logo.png",
Copy link
Contributor

Choose a reason for hiding this comment

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

should this logo url have http:// on the front? doesn't seem to load for me without it.

Copy link
Member Author

Choose a reason for hiding this comment

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

are you loading it via file:///? That won't work then. But we could potentially just do http://

Copy link
Contributor

Choose a reason for hiding this comment

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

without http:// on the front chrome seems to default to file://. Even with that when I open the generated index.html file the logo is not loaded.

Uses the TUI JSDoc template. All configuring of jsdoc was moved to the jsdoc.json file.
There's currently a bug with TUI regarding the tutorials (nhn/tui.jsdoc-template#12) but it should via gh-pages currently because it'll now autorender markdown into html.
@gkatsev gkatsev merged commit e642295 into videojs:master Jan 24, 2017
@gkatsev gkatsev deleted the jsdoc-template branch January 24, 2017 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
6.0
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants