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

Documentation: Try a new custom documentation tool to rule them all #1786

Merged
merged 56 commits into from Jul 10, 2017

Conversation

Projects
None yet
5 participants
@youknowriad
Contributor

youknowriad commented Jul 7, 2017

Requirements we're trying to achieve here:

  • A unique documentation tool for all our docs
  • Contains the creating blocks guide
  • Code examples are written in ES5 and ESnext.
  • Has to live in the repository
  • Integrate Well with the WordPress.org docs
  • Will contain the pattern library
    • Loads and runs React Components
    • Can change components' props live
  • Will be searchable
  • Can support advanced documentation (for example Redux's selectors discovery)

This PR adds a Docs tool called docutron you can launch like that:

npm run docs-start

The docs folder has an index.js as an Entry point where you can add "stories" to your documentation website:

importaddStory } from 'glutenberg';
import myReadme from '../readme.md';
addStory( {
  name: 'intro',
  title: 'Introduction',
  markdown: myReadme,
} );

For more flexibility, a story can provide a React Component instead of providing a markdown string.

screen shot 2017-07-07 at 17 33 21

@youknowriad youknowriad self-assigned this Jul 7, 2017

@youknowriad youknowriad requested a review from aduth Jul 7, 2017

@nylen

This comment has been minimized.

Show comment
Hide comment
@nylen

nylen Jul 7, 2017

Member

Integrate Well with the WordPress.org docs

This is not something to solve today. However, what is the current plan for uploading this documentation to developer.wordpress.org? How do you see this integration going?

When we do that, let's clean up code samples on that site. Right now the font size is so big that they are almost impossible to usefully read.

Member

nylen commented Jul 7, 2017

Integrate Well with the WordPress.org docs

This is not something to solve today. However, what is the current plan for uploading this documentation to developer.wordpress.org? How do you see this integration going?

When we do that, let's clean up code samples on that site. Right now the font size is so big that they are almost impossible to usefully read.

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Jul 7, 2017

Contributor

This is not something to solve today. However, what is the current plan for uploading this documentation to developer.wordpress.org? How do you see this integration going?

Today, we're trying to just have a similar styling, and maybe we just embed an iframe in the WordPress docs since it's have the same styling.

Contributor

youknowriad commented Jul 7, 2017

This is not something to solve today. However, what is the current plan for uploading this documentation to developer.wordpress.org? How do you see this integration going?

Today, we're trying to just have a similar styling, and maybe we just embed an iframe in the WordPress docs since it's have the same styling.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 8, 2017

Coverage Status

Changes Unknown when pulling f9e8caa on try/new-docs-tool into ** on master**.

Coverage Status

Changes Unknown when pulling f9e8caa on try/new-docs-tool into ** on master**.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 8, 2017

Coverage Status

Changes Unknown when pulling 01d229c on try/new-docs-tool into ** on master**.

Coverage Status

Changes Unknown when pulling 01d229c on try/new-docs-tool into ** on master**.

@youknowriad youknowriad changed the title from [WIP] Documentation: Try a new custom documentation tool to rule them all to Documentation: Try a new custom documentation tool to rule them all Jul 8, 2017

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Jul 8, 2017

Contributor

The documentation tool is almost ready. I need some eyes here cc @aduth
I wonder if we should merge this without completing the blocks guide.
Also, once merged, we'll have to PR and remove Storybook and update the deployment hook to deploy this instead.

Contributor

youknowriad commented Jul 8, 2017

The documentation tool is almost ready. I need some eyes here cc @aduth
I wonder if we should merge this without completing the blocks guide.
Also, once merged, we'll have to PR and remove Storybook and update the deployment hook to deploy this instead.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 8, 2017

Coverage Status

Changes Unknown when pulling debed01 on try/new-docs-tool into ** on master**.

Coverage Status

Changes Unknown when pulling debed01 on try/new-docs-tool into ** on master**.

Show outdated Hide outdated docs-tool/src/components/Page.js
return (
<div>
<div>

This comment has been minimized.

@aduth

aduth Jul 8, 2017

Member

Do we need the nested div ?

@aduth

aduth Jul 8, 2017

Member

Do we need the nested div ?

Show outdated Hide outdated docs-tool/src/components/Page.js
}
componentDidUpdate() {
Prism.highlightAll();

This comment has been minimized.

@aduth

aduth Jul 8, 2017

Member

Can we be more targeted about this? Either passing the individual node or the code content?

@aduth

aduth Jul 8, 2017

Member

Can we be more targeted about this? Either passing the individual node or the code content?

This comment has been minimized.

@youknowriad

youknowriad Jul 8, 2017

Contributor

Going to leave this for now, because we technically don't know if there's a code content in the current tabs, these tabs can contain any markup.

@youknowriad

youknowriad Jul 8, 2017

Contributor

Going to leave this for now, because we technically don't know if there's a code content in the current tabs, these tabs can contain any markup.

Show outdated Hide outdated docs-tool/src/components/Page.js
class Page extends Component {
componentDidMount() {
Prism.highlightAll();

This comment has been minimized.

@aduth

aduth Jul 8, 2017

Member

Seems odd to call code highlighting here, and not in the tabs.

@aduth

aduth Jul 8, 2017

Member

Seems odd to call code highlighting here, and not in the tabs.

This comment has been minimized.

@youknowriad

youknowriad Jul 8, 2017

Contributor

This is necessary when you switch pages that contain code blocks without tabs

@youknowriad

youknowriad Jul 8, 2017

Contributor

This is necessary when you switch pages that contain code blocks without tabs

Show outdated Hide outdated docs-tool/src/components/Page.js
<div className="navigation">
{ !! previousStory && (
<p className="nav-older">
<Link to={ previousStory.path }>{ '' } { previousStory.title }</Link>

This comment has been minimized.

@aduth

aduth Jul 8, 2017

Member

Does need to be wrapped as an expression?

@aduth

aduth Jul 8, 2017

Member

Does need to be wrapped as an expression?

This comment has been minimized.

@aduth

aduth Jul 8, 2017

Member

The core docs include some extra hints here, like rel="previous" and rel="next".

https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types

@aduth

aduth Jul 8, 2017

Member

The core docs include some extra hints here, like rel="previous" and rel="next".

https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types

This comment has been minimized.

@youknowriad

youknowriad Jul 8, 2017

Contributor

The wrapping makes it more visible in my IDE. The same sign is used to display "tabs". But I can live without it if necessary

@youknowriad

youknowriad Jul 8, 2017

Contributor

The wrapping makes it more visible in my IDE. The same sign is used to display "tabs". But I can live without it if necessary

Show outdated Hide outdated docs-tool/.gitignore
@@ -0,0 +1,21 @@
# See https://help.github.com/ignore-files/ for more about ignoring files.

This comment has been minimized.

@aduth

aduth Jul 8, 2017

Member

Should docs-tool be a top-level folder, or could we move it within docs?

@aduth

aduth Jul 8, 2017

Member

Should docs-tool be a top-level folder, or could we move it within docs?

This comment has been minimized.

@youknowriad

youknowriad Jul 8, 2017

Contributor

I think we should move it outside the repository later, glutenberg maybe :)

@youknowriad

youknowriad Jul 8, 2017

Contributor

I think we should move it outside the repository later, glutenberg maybe :)

Show outdated Hide outdated docs-tool/bin/helpers/extend-webpack-config.js
// Adding the markdown loader and exclude if from the file loader
webpackConfig.module.rules.forEach( rule => {
if ( rule.loader === require.resolve( 'file-loader' ) ) {

This comment has been minimized.

@aduth

aduth Jul 8, 2017

Member

Should loaders be defined in docs-tool/package.json if we're treating it as a separate project, or are these guaranteed to exist by create-react-app?

@aduth

aduth Jul 8, 2017

Member

Should loaders be defined in docs-tool/package.json if we're treating it as a separate project, or are these guaranteed to exist by create-react-app?

This comment has been minimized.

@youknowriad

youknowriad Jul 8, 2017

Contributor

I think CRA guarantee this since it's using it.

@youknowriad

youknowriad Jul 8, 2017

Contributor

I think CRA guarantee this since it's using it.

Show outdated Hide outdated docs-tool/src/components/Page.js
constructor() {
super( ...arguments );
this.state = {
activeTab: 0,

This comment has been minimized.

@aduth

aduth Jul 8, 2017

Member

A future / present feature which would be nice to support is remembering preference across each page.

@aduth

aduth Jul 8, 2017

Member

A future / present feature which would be nice to support is remembering preference across each page.

This comment has been minimized.

@youknowriad

youknowriad Jul 8, 2017

Contributor

The problem here is that the tabs can be anything not always (es5, es6)

@youknowriad

youknowriad Jul 8, 2017

Contributor

The problem here is that the tabs can be anything not always (es5, es6)

Show outdated Hide outdated docs-tool/src/components/Sidebar.js
return (
<div id="secondary" className="widget-area">
<div className="secondary-content">
<aside id="handbook_pages-3" className="widget widget_wporg_handbook_pages">

This comment has been minimized.

@aduth

aduth Jul 8, 2017

Member

Think we can drop the id.

@aduth

aduth Jul 8, 2017

Member

Think we can drop the id.

Show outdated Hide outdated docs-tool/src/index.js
@@ -0,0 +1,13 @@
import React from 'react';
import ReactDOM from 'react-dom';
import 'prismjs';

This comment has been minimized.

@aduth

aduth Jul 8, 2017

Member

Can we see if we can get rid of this? Not sure why we need both this and Prism.highlightAll(). Shouldn't highlightAll only be occurring when the code has been rendered anyways?

@aduth

aduth Jul 8, 2017

Member

Can we see if we can get rid of this? Not sure why we need both this and Prism.highlightAll(). Shouldn't highlightAll only be occurring when the code has been rendered anyways?

Show outdated Hide outdated docs/stories/index.js
@@ -0,0 +1,47 @@
importaddStory } from 'glutenberg';

This comment has been minimized.

@aduth

aduth Jul 8, 2017

Member

Will we have more than one file in this folder? Should we start as docs/stories.js to avoid prematurely nesting directories?

@aduth

aduth Jul 8, 2017

Member

Will we have more than one file in this folder? Should we start as docs/stories.js to avoid prematurely nesting directories?

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 8, 2017

Coverage Status

Changes Unknown when pulling 830c99a on try/new-docs-tool into ** on master**.

Coverage Status

Changes Unknown when pulling 830c99a on try/new-docs-tool into ** on master**.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 8, 2017

Coverage Status

Changes Unknown when pulling 830c99a on try/new-docs-tool into ** on master**.

Coverage Status

Changes Unknown when pulling 830c99a on try/new-docs-tool into ** on master**.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 10, 2017

Coverage Status

Changes Unknown when pulling 2455a66 on try/new-docs-tool into ** on master**.

Coverage Status

Changes Unknown when pulling 2455a66 on try/new-docs-tool into ** on master**.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 10, 2017

Coverage Status

Coverage remained the same at 17.816% when pulling 1295aa9 on try/new-docs-tool into d157b5e on master.

Coverage Status

Coverage remained the same at 17.816% when pulling 1295aa9 on try/new-docs-tool into d157b5e on master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 10, 2017

Coverage Status

Coverage remained the same at 17.816% when pulling 1295aa9 on try/new-docs-tool into d157b5e on master.

Coverage Status

Coverage remained the same at 17.816% when pulling 1295aa9 on try/new-docs-tool into d157b5e on master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 10, 2017

Coverage Status

Coverage remained the same at 17.816% when pulling 0748c49 on try/new-docs-tool into d157b5e on master.

Coverage Status

Coverage remained the same at 17.816% when pulling 0748c49 on try/new-docs-tool into d157b5e on master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 10, 2017

Coverage Status

Coverage remained the same at 17.816% when pulling 2adf01f on try/new-docs-tool into d157b5e on master.

Coverage Status

Coverage remained the same at 17.816% when pulling 2adf01f on try/new-docs-tool into d157b5e on master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 10, 2017

Coverage Status

Coverage remained the same at 17.816% when pulling 4a4a749 on try/new-docs-tool into d157b5e on master.

Coverage Status

Coverage remained the same at 17.816% when pulling 4a4a749 on try/new-docs-tool into d157b5e on master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 10, 2017

Coverage Status

Coverage remained the same at 17.816% when pulling 817ad44 on try/new-docs-tool into 4e0ea5c on master.

Coverage Status

Coverage remained the same at 17.816% when pulling 817ad44 on try/new-docs-tool into 4e0ea5c on master.

youknowriad and others added some commits Jul 10, 2017

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 10, 2017

Coverage Status

Coverage remained the same at 17.816% when pulling a308cd1 on try/new-docs-tool into 4e0ea5c on master.

Coverage Status

Coverage remained the same at 17.816% when pulling a308cd1 on try/new-docs-tool into 4e0ea5c on master.

Show outdated Hide outdated docutron/bin/helpers/extend-webpack-config.js
const path = require( 'path' );
module.exports = function( webpackConfig, usersCwd ) {
const docsFolder = 'docs';

This comment has been minimized.

@aduth

aduth Jul 10, 2017

Member

Should this be hard-coded as docs, or should we support specifying this from the command-line?

@aduth

aduth Jul 10, 2017

Member

Should this be hard-coded as docs, or should we support specifying this from the command-line?

Show outdated Hide outdated docutron/bin/cli.js
@@ -0,0 +1,14 @@
#!/usr/bin/env node
const path = require( 'path' );

This comment has been minimized.

@aduth

aduth Jul 10, 2017

Member

Should we add DocBlocks for dependencies ("External dependencies", etc)? For consistency with other "modules".

@aduth

aduth Jul 10, 2017

Member

Should we add DocBlocks for dependencies ("External dependencies", etc)? For consistency with other "modules".

Show outdated Hide outdated docutron/src/components/Page.js
import React, { Component } from 'react';
import { Link } from 'react-router-dom';
import { getNextStory, getPreviousStory } from 'docutron';

This comment has been minimized.

@aduth

aduth Jul 10, 2017

Member

It seems a bit odd for this module to depend from itself. Should we instead import by traversing to the relative path?

@aduth

aduth Jul 10, 2017

Member

It seems a bit odd for this module to depend from itself. Should we instead import by traversing to the relative path?

Show outdated Hide outdated docutron/src/components/Page.js
import markdown from '../markdown';
import Tabs from './Tabs';
function MarkdownContent( { content } ) {

This comment has been minimized.

@aduth

aduth Jul 10, 2017

Member

We might consider extracting this component to a separate file.

@aduth

aduth Jul 10, 2017

Member

We might consider extracting this component to a separate file.

Show outdated Hide outdated docutron/src/index.js
@@ -0,0 +1,12 @@
import React from 'react'; // eslint-disable-line no-unused-vars

This comment has been minimized.

@aduth

aduth Jul 10, 2017

Member

We shouldn't need eslint-disable-line no-unused-vars anymore.

@aduth

aduth Jul 10, 2017

Member

We shouldn't need eslint-disable-line no-unused-vars anymore.

youknowriad and others added some commits Jul 10, 2017

Specify Docutron version
🤖 Recalibrating mainframe...
Remove service worker registration
May be "nice to have", but not intending to support or maintain
Rename extendsConfig to extendConfig
Consistency with named file
Prevent search form submission
Filter results updated live
@aduth

aduth approved these changes Jul 10, 2017

This appears to be in a good state for an initial pass at documentation. Let's get this in and continue iterating.

@youknowriad youknowriad merged commit 7779f79 into master Jul 10, 2017

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 17.816%
Details

@youknowriad youknowriad deleted the try/new-docs-tool branch Jul 10, 2017

@karmatosed

This comment has been minimized.

Show comment
Hide comment
@karmatosed

karmatosed Jul 17, 2017

Member

Just checking on this, we have 2 issues for the pattern library here: https://github.com/WordPress/gutenberg/labels/Pattern%20Library. Is the idea that this works as that? I am trying to get context and don't want to duplicate if this is already happening.

Member

karmatosed commented Jul 17, 2017

Just checking on this, we have 2 issues for the pattern library here: https://github.com/WordPress/gutenberg/labels/Pattern%20Library. Is the idea that this works as that? I am trying to get context and don't want to duplicate if this is already happening.

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Jul 18, 2017

Member

@karmatosed I think the work here relates to the pattern library, but doesn't necessary supersede it. Rather, this merely sets a base for consolidating documentation, which we can build out to include a components library similar to Calypso DevDocs, homegrown in the same fashion. The problem with Storybook is that it worked particularly well at this one role of component demonstrations, but wouldn't allow us to scale up to include more forms of documentation, or style in a fashion that would be easy to integrate with the WordPress developer site.

Member

aduth commented Jul 18, 2017

@karmatosed I think the work here relates to the pattern library, but doesn't necessary supersede it. Rather, this merely sets a base for consolidating documentation, which we can build out to include a components library similar to Calypso DevDocs, homegrown in the same fashion. The problem with Storybook is that it worked particularly well at this one role of component demonstrations, but wouldn't allow us to scale up to include more forms of documentation, or style in a fashion that would be easy to integrate with the WordPress developer site.

@karmatosed

This comment has been minimized.

Show comment
Hide comment
@karmatosed

karmatosed Jul 18, 2017

Member

@aduth thank you for explaining a little. I am really interested in anything that makes things easier :) How can we unite the approaches? I really would like to do that. Maybe we can take it to those tickets though, if that is more suitable?

Member

karmatosed commented Jul 18, 2017

@aduth thank you for explaining a little. I am really interested in anything that makes things easier :) How can we unite the approaches? I really would like to do that. Maybe we can take it to those tickets though, if that is more suitable?

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Jul 18, 2017

Member

Yes, let's move discussion into issues. I've left another comment at #1706 (comment)

Member

aduth commented Jul 18, 2017

Yes, let's move discussion into issues. I've left another comment at #1706 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment