-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
jsdoc output #3910
Conversation
Do we want to do custom styling for video.js in another PR? |
Can we change the logo in the top left? (from toastui to videojs) |
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. |
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) |
I also noticed that examples seem to be cut off at the bottom? (might be a bug in tui) |
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? |
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 |
I also noticed that the API link at the top doesn't just link to the current page. |
Ah, I think that's because it's just inlining the |
The latest updates depend on these changes: https://github.com/nhnent/tui.jsdoc-template/pulls/gkatsev |
b701443
to
69f68fa
Compare
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", |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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://
There was a problem hiding this comment.
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.
69f68fa
to
44a24b9
Compare
This reverts commit 58f2349.
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: