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

Fix Broken Link; Misc Grammar Fixes; Indentation Fix #18423

Merged
merged 1 commit into from Dec 20, 2019

Conversation

@dhurlburtusa
Copy link
Contributor

dhurlburtusa commented Nov 11, 2019

Description

Fix Broken Link; Misc Grammar Fixes; Indentation Fix

It looks like the link was trying to point to https://developer.wordpress.org/block-editor/tutorials/javascript/loading-javascript/. (Was pointing to loading-javascript.md.) But I think it would be better to point to https://developer.wordpress.org/block-editor/tutorials/javascript/extending-the-block-editor/ instead which has the correct "Step 2" mentioned in the link text. The "Step 2" also includes the dependencies array for the code.

@@ -119,13 +119,13 @@ To configure npm to run a script, you use the scripts section in `package.json`

You can then run the build using: `npm run build`.

After the build finishes, you will see the built file created at `build/index.js`. Enqueue this file in the admin screen as you would any JavaScript in WordPress, see [Step 2 in this tutorial](loading-javascript.md), and the block will load in the editor.
After the build finishes, you will see the built file created at `build/index.js`. Enqueue this file in the admin screen as you would any JavaScript in WordPress, see [Step 2 in this tutorial](extending-the-block-editor/), and the block will load in the editor.

This comment has been minimized.

Copy link
@jorgefilipecosta

jorgefilipecosta Nov 12, 2019

Member

Hi @dhurlburtusa thank you for your contribution 👍 After checking both links, It seems the previous link was correct, it was linking to step two linked on https://github.com/WordPress/gutenberg//blob/0ad0414e7a469254ef1ad105fdcf14248a986686/docs/designers-developers/developers/tutorials/javascript/readme.md#L15. And the previous text refers the mechanism to enqueue which is documented on loading-javascript.md.

This comment has been minimized.

Copy link
@dhurlburtusa

dhurlburtusa Nov 12, 2019

Author Contributor

Thanks @jorgefilipecosta for confirming the correct destination. I wasn't sure what step was being referred to when it said "Step 2 in this tutorial".

I am confused on the best way to fix this link. Should it be changed to point to readme.md since the link text ("Step 2 in this tutorial") refers to the tutorial itself? Or should we link directly to step 2 which would be loading-javascript.md. In that case, I think the link text should be updated.

There is also some confusion as to whether I should be leaving the extension on (.md). In GitHub, it seems the extension is necessary. Otherwise navigating within GitHub is broken. But when I go to the hosted website version, none of the extensions exist. I guess some build process removes them. And therein lies the problem. Here is the file structure:

...
js-build-setup.md
loading-javascript.md
...
readme.md
...

The problem is that in GitHub, js-build-setup.md and loading-javascript.md are sibling files. So linking with loading-javascript.md works correctly. But on the web side of things, they are sibling paths. So linking to ../loading-javascript/ is required.

I've seen in https://github.com/WordPress/gutenberg/blob/master/docs/designers-developers/developers/tutorials/javascript/loading-javascript.md that some links take you out of GitHub and to the website (developer.wordpress.org). But in other places such as https://github.com/WordPress/gutenberg/blob/master/docs/designers-developers/developers/tutorials/javascript/readme.md uses relative references to the markdown file.

So, I am not sure the recommended way to fix this link.

This comment has been minimized.

Copy link
@jorgefilipecosta

jorgefilipecosta Nov 18, 2019

Member

Hi @dhurlburtusa,

I am confused on the best way to fix this link. Should it be changed to point to readme.md since the link text ("Step 2 in this tutorial") refers to the tutorial itself? Or should we link directly to step 2 which would be loading-javascript.md. In that case, I think the link text should be updated.

I guess we should link to loading javascript and that it makes sense to update the link text.

The problem is that in GitHub, js-build-setup.md and loading-javascript.md are sibling files. So linking with loading-javascript.md works correctly. But on the web side of things, they are sibling paths. So linking to ../loading-javascript/ is required.

Interesting point. Would link to something like ../../javascript/loading-javascript.md work on GitHub and hosted website version?

@@ -119,13 +119,13 @@ To configure npm to run a script, you use the scripts section in `package.json`

You can then run the build using: `npm run build`.

After the build finishes, you will see the built file created at `build/index.js`. Enqueue this file in the admin screen as you would any JavaScript in WordPress, see [Step 2 in this tutorial](loading-javascript.md), and the block will load in the editor.
After the build finishes, you will see the built file created at `build/index.js`. Enqueue this file in the admin screen as you would any JavaScript in WordPress, see [Step 2 in this tutorial](readme.md), and the block will load in the editor.

This comment has been minimized.

Copy link
@mkaz

mkaz Nov 23, 2019

Member

This link was changed in a separate PR, #18446

Can you rebase so we can accept the additional changes? Thanks

This comment has been minimized.

Copy link
@jorgefilipecosta

jorgefilipecosta Dec 17, 2019

Member

Hi @mkaz I rebased and applied the suggested change.

This comment has been minimized.

Copy link
@dhurlburtusa

dhurlburtusa Dec 19, 2019

Author Contributor

@mkaz, @jorgefilipecosta Is there anything you guys need from me?

…mit)

Squashed commits:
[4fd170c885] Fix Broken Link; Misc Grammar Fixes; Indentation Fix

It looks like the link was trying to point to https://developer.wordpress.org/block-editor/tutorials/javascript/loading-javascript/. But I think is would be better to point to https://developer.wordpress.org/block-editor/tutorials/javascript/extending-the-block-editor/ which has the correct "Step 2" which also includes the dependencies array for the code.
@jorgefilipecosta jorgefilipecosta force-pushed the dhurlburtusa:patch-1 branch from aa3ae65 to 841c77a Dec 17, 2019
@mkaz
mkaz approved these changes Dec 20, 2019
Copy link
Member

mkaz left a comment

Thanks for the update 👍

@mkaz mkaz merged commit 0415ee8 into WordPress:master Dec 20, 2019
2 checks passed
2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@youknowriad youknowriad added this to the Gutenberg 7.2 milestone Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.