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

Add documentation for the RichText component to the Block Editor Handbook #15956

Merged
merged 5 commits into from Jul 11, 2019

Conversation

@jg314
Copy link
Contributor

commented Jun 2, 2019

Description

Add RichText component documentation to the Block Editor Handbook. Currently all that exists is the Github documentation at https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/components/rich-text/README.md.

How has this been tested?

I did not test this outside of the basic code example provided in the content. If there is other testing that's needed please let me know.

Types of changes

  1. Create a new RichtText Reference page that lives under the Developer Documentation section of the handbook.
  2. Modify the table of contents to include the new page.

Other Notes

  • This content was recommended in #14528 by @noisysocks.
  • I attempted to run npm run docs:build as noted on https://github.com/WordPress/gutenberg/blob/master/docs/contributors/document.md, but I continued to get an error within terminal. Let me know if it would be helpful for me to troubleshoot that.
  • Most importantly, this is the first time I've ever contributed, so I apologize in advance if I missed something. Please let me know if there are ways to improve what's here. I'd love and welcome any feedback.
@noisysocks

This comment has been minimized.

Copy link
Member

commented Jun 3, 2019

Hey @jg314! Thanks so much for working on this!

I attempted to run npm run docs:build as noted on https://github.com/WordPress/gutenberg/blob/master/docs/contributors/document.md, but I continued to get an error within terminal. Let me know if it would be helpful for me to troubleshoot that.

You'll need to generate docs and commit the files that it generates so that Travis CI is happy with this branch and lets us merge it.

What error are you receiving when you run npm run docs:build?

@noisysocks noisysocks requested a review from ellatrix Jun 3, 2019

@jg314

This comment has been minimized.

Copy link
Contributor Author

commented Jun 3, 2019

@noisysocks, thanks for taking such a quick look at this PR. I've pasted the error I'm getting when trying to run npm run docs:build below:

11 silly lifecycle gutenberg@5.8.0~docs:build: Returned: code: 1  signal: null
12 info lifecycle gutenberg@5.8.0~docs:build: Failed to exec docs:build script
13 verbose stack Error: gutenberg@5.8.0 docs:build: `node ./docs/tool && node ./bin/update-readmes`
13 verbose stack Exit status 1
13 verbose stack     at EventEmitter.<anonymous> (C:\Program Files\nodejs\node_modules\npm\node_modules\npm-lifecycle\index.js:285:16)
13 verbose stack     at emitTwo (events.js:126:13)
13 verbose stack     at EventEmitter.emit (events.js:214:7)
13 verbose stack     at ChildProcess.<anonymous> (C:\Program Files\nodejs\node_modules\npm\node_modules\npm-lifecycle\lib\spawn.js:55:14)
13 verbose stack     at emitTwo (events.js:126:13)
13 verbose stack     at ChildProcess.emit (events.js:214:7)
13 verbose stack     at maybeClose (internal/child_process.js:925:16)
13 verbose stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:209:5)
14 verbose pkgid gutenberg@5.8.0
15 verbose cwd C:\Users\username_removed\Desktop\gutenberg
16 verbose Windows_NT 10.0.17763
17 verbose argv "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "run" "docs:build"
18 verbose node v8.11.3
19 verbose npm  v5.6.0
20 error code ELIFECYCLE
21 error errno 1
22 error gutenberg@5.8.0 docs:build: `node ./docs/tool && node ./bin/update-readmes`
22 error Exit status 1
23 error Failed at the gutenberg@5.8.0 docs:build script.
23 error This is probably not a problem with npm. There is likely additional logging output above.
24 verbose exit [ 1, true ]

Have you all seen this before? Any idea what I can do to fix it?

@nosolosw

This comment has been minimized.

Copy link
Member

commented Jun 4, 2019

This is a great addition!

The RichText component API is exposed in the handbook as part of the block-editor package. See https://developer.wordpress.org/block-editor/packages/packages-block-editor/#RichText

Unfortunately, it is undocumented and points to its README. This can be fixed by updating the source JSDoc comment and the changes will be reflected in the handbook.

That'd be a great separate PR that would play well with the good work on this one :)

@jg314

This comment has been minimized.

Copy link
Contributor Author

commented Jun 4, 2019

Thanks for the feedback @nosolosw. It sounds like once we've figured out the issue with npm run docs:build and solidified the plans for these relative vs. absolute URLs we should be good to move forward.

@jg314

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2019

The issue I mentioned above regarding npm run docs:build seems to be related to execSync( join( __dirname, 'update-data.js' ) ); within wp-content\plugins\gutenberg\docs\tool\index.js. I'm on a Windows machine and spaces in directory names are causing the error. Any suggestions on how to deal with that? Thanks a ton for the help.

@noisysocks

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

Hey @jg314, it's likely a bug in the docs tool. I'm setting up a Windows environment and looking into it now 🙂

@noisysocks

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

Hi @jg314, I opened #16029 which should fix the bug you're running into. Once my PR is merged you ought to be able to rebase your PR, generate docs, and get Travis CI to pass! 😅

jg314 and others added some commits Jun 8, 2019

Modify link to RichText properties to make it absolute
Co-Authored-By: Robert Anderson <robert@noisysocks.com>
@jg314

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2019

The necessary URLs have been updated to be absolute. Once I'm able to run npm run docs:build without error we should be set to move this forward.

Thanks for all the help on this so far everyone.

@gziolo

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

The necessary URLs have been updated to be absolute.

All URLs works properly, I'm glad we managed to clarify what's the proper approach. It's still not ideal but at least we have an agreement :)

@jg314

This comment has been minimized.

Copy link
Contributor Author

commented Jun 15, 2019

I spent some more time today trying to run npm run docs:build with no luck. I attempted to work through the issues noted here and in #16029, but I'm still getting errors when I run the command. Does anyone have any suggestions on how to move this forward? I'm absolutely open to a different approach. Thanks for the help.

@jg314

This comment has been minimized.

Copy link
Contributor Author

commented Jun 15, 2019

I was finally able to get npm run docs:build to work by following these steps:

  1. Moving the gutenberg plugin folder to another location (I did this because one of the directories where the code lives had a space in it, causing the build to fail)
  2. Installing the Windows Subsystem for Linux
  3. Running npm install --save-dev react-docgen (I did this because I was getting an error that said .../node_modules/.bin/docgen: not found)

Once it runs a lot of .md files are modified to remove documentation and replace it with "Undocumented declaration." That doesn't seem right, so I wanted to check before committing all of these files. Please let me know how you'd prefer I handle this. Thanks a ton.

@jg314

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2019

Hey @noisysocks and @gziolo, is there anything else I can do to move this forward? Thanks again for all the help on this front.

@noisysocks

This comment has been minimized.

Copy link
Member

commented Jul 4, 2019

Hey @jg314, sorry for the delay—I've been away on vacation!

Once it runs a lot of .md files are modified to remove documentation and replace it with "Undocumented declaration." That doesn't seem right, so I wanted to check before committing all of these files.

No, that doesn't seem right 😕

This should be the result of git diff after you run npm run docs:build:

diff --git a/docs/manifest-devhub.json b/docs/manifest-devhub.json
index f762d1422..754744fcd 100644
--- a/docs/manifest-devhub.json
+++ b/docs/manifest-devhub.json
@@ -137,6 +137,12 @@
                "markdown_source": "../docs/designers-developers/developers/slotfills/plugin-sidebar-more-menu-item.md",
                "parent": "slotfills"
        },
+       {
+               "title": "RichText Reference",
+               "slug": "richtext",
+               "markdown_source": "../docs/designers-developers/developers/richtext.md",
+               "parent": "developers"
+       },
        {
                "title": "Internationalization",
                "slug": "internationalization",

Let's try to get everything fixed up over in #16029 and then we can continue with this PR 🙂 — thanks for your patience!

@jg314

This comment has been minimized.

Copy link
Contributor Author

commented Jul 5, 2019

Thanks for getting back to me @noisysocks. I really appreciate it. And that sounds good, let's work on #16029 first and then we'll get to this one.

@jg314

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

After the awesome work @noisysocks did on #16029 I was able to commit up the modified manifest.json file and the checks have passed. @noisysocks, is there anything else I can do to help at this point?

@noisysocks

This comment has been minimized.

Copy link
Member

commented Jul 11, 2019

Thanks for your patience @jg314 and for contributing some documentation! This looks like a great start to get in and iterate on 🙂

@noisysocks noisysocks merged commit c23228f into WordPress:master Jul 11, 2019

1 check passed

Travis CI - Pull Request Build Passed
Details

@jg314 jg314 deleted the jg314:docs/rich-text branch Jul 11, 2019

@jg314

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2019

Awesome, I'm happy to see it live. Thanks again for all the help @noisysocks. I really appreciate it.

Tug added a commit that referenced this pull request Jul 12, 2019

Add documentation for the RichText component to the Block Editor Hand…
…book (#15956)

* add documentation for the RichText component

* modify internal links to use the appropriate format

* Modify link to RichText properties to make it absolute

Co-Authored-By: Robert Anderson <robert@noisysocks.com>

* Modify links to core block source code to make them absolute URLs

* Update manifest.json by running npm run docs:build

jg314 added a commit to jg314/gutenberg that referenced this pull request Jul 19, 2019

Add documentation for the RichText component to the Block Editor Hand…
…book (WordPress#15956)

* add documentation for the RichText component

* modify internal links to use the appropriate format

* Modify link to RichText properties to make it absolute

Co-Authored-By: Robert Anderson <robert@noisysocks.com>

* Modify links to core block source code to make them absolute URLs

* Update manifest.json by running npm run docs:build

@youknowriad youknowriad added this to the Gutenberg 6.2 milestone Jul 26, 2019

sbardian added a commit to sbardian/gutenberg that referenced this pull request Jul 29, 2019

Add documentation for the RichText component to the Block Editor Hand…
…book (WordPress#15956)

* add documentation for the RichText component

* modify internal links to use the appropriate format

* Modify link to RichText properties to make it absolute

Co-Authored-By: Robert Anderson <robert@noisysocks.com>

* Modify links to core block source code to make them absolute URLs

* Update manifest.json by running npm run docs:build
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.