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

DevDocs: remove Gutenberg components and blocks #32026

Merged
merged 4 commits into from
Apr 10, 2019
Merged

Conversation

vindl
Copy link
Member

@vindl vindl commented Apr 4, 2019

Changes proposed in this Pull Request

Following the removal of Gutenlypso (#31416), it would probably be best to remove these examples to avoid confusion.

See discussion in p7jreA-2g7 (#comment-1869)

Removes the following packages:

  • @wordpress/block-library
  • @wordpress/date
  • @wordpress/hooks

Testing instructions

  • Smoke test Calypso DevDocs.
  • There should be no Gutenberg Components and Blocks items in the sidebar.

@vindl vindl added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Type] Janitorial [Goal] Gutenberg Working towards full integration with Gutenberg labels Apr 4, 2019
@vindl vindl self-assigned this Apr 4, 2019
@matticbot
Copy link
Contributor

@vindl vindl requested a review from a team April 4, 2019 14:42
@vindl vindl force-pushed the remove/gutenlypso-devdocs branch from 8105484 to 1a51094 Compare April 4, 2019 15:37
@vindl vindl requested a review from a team as a code owner April 4, 2019 15:37
@@ -93,9 +88,7 @@ const Collection = ( {
component={ component }
section={ section }
/>
{ component && (
<ReadmeViewer readmeFilePath={ readmeFilePath } showEditLink={ showEditLink } />
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that the showEditLink prop has been added in the same PR that added the whole Gutenberg components in the DevDocs (#26314), and is not used anywhere else.
I guess we could remove it from ReadmeViewer (and its tests) as well.

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 guess we could remove it from ReadmeViewer (and its tests) as well.

Hmm, I still see a couple its usages that are not related to Gutenberg components and blocks: 🤔
https://github.com/Automattic/wp-calypso/search?p=1&q=%22ReadmeViewer%22&unscoped_q=%22ReadmeViewer%22

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I meant remove the showEditLink prop from ReadmeViewer (and its tests), not the entire component 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh sorry, it seems I wasn't fully awake while reading your comment. 🤦‍♂️ That's exactly what you wrote the first time around, but I somehow managed to completely misunderstand it. 😄
Anyway, I'll clean it up in a follow-up.

Copy link
Contributor

@Copons Copons left a comment

Choose a reason for hiding this comment

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

I've left two comments: one is a nitpick, the other a blocker. Both should be easily fixable though!

Otherwise, this tests well!

(Also I think we need to rebase and re-shrinkwrap.)

.main.devdocs.devdocs__gutenberg-components {
// Make way for showing big components and multiple columns
max-width: 100%;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You've deleted too much stuff here!
We want to keep the max-width: 100% for .devdocs__blocks and .devdocs__components, not only because right now we are giving them the same rules as the next selector, but also because we want the docs to be nice and large! 😛

Jokes aside, you can see that something is especially wrong in the Playground (/devdocs/playground), where the code editor is just too narrow to be usable.
Though, the error is visible in Blocks (/devdocs/blocks) and UI Components (/devdocs/design) as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ooops! Nice catch! Restored this in 2673416c2f122b1d5e91299d864ea48bf5ad2ed1. :)

@vindl vindl force-pushed the remove/gutenlypso-devdocs branch from 1a51094 to 6f1fa9e Compare April 9, 2019 09:00
@Copons
Copy link
Contributor

Copons commented Apr 9, 2019

I've approved this because ReadmeViewer.showEditLink is a nitpick that we can take care in another, simpler PR.

Following the removal of Gutenlypso, it would probably be best
to remove these examples to avoid confusion.
@vindl vindl force-pushed the remove/gutenlypso-devdocs branch from 6f1fa9e to 7f15677 Compare April 10, 2019 11:03
@vindl vindl merged commit 4b8998a into master Apr 10, 2019
@vindl vindl deleted the remove/gutenlypso-devdocs branch April 10, 2019 14:36
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 10, 2019
@sarayourfriend sarayourfriend restored the remove/gutenlypso-devdocs branch September 9, 2020 16:42
@sirreal sirreal deleted the remove/gutenlypso-devdocs branch November 27, 2020 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Goal] Gutenberg Working towards full integration with Gutenberg [Type] Janitorial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants