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

Move block server-side logic into same directory as client-side logic #2014

Merged
merged 4 commits into from
Jul 31, 2017

Conversation

westonruter
Copy link
Member

When working with dynamic blocks it is not ideal for the JS and PHP logic to be separated out in two separate locations. Ideally the dynamic block server rendering logic as well as unit tests could be placed right inside of the same directory as the client logic.

This would also impact the Latest Comments block: #1931.

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

See also: #1095

We need to remove this line:

https://github.com/WordPress/gutenberg/blob/44ad46a/bin/build-plugin-zip.sh#L71

And ensure that block PHP files are included in the generated plugin.

@westonruter westonruter added the [Status] In Progress Tracking issues with work in progress label Jul 26, 2017
@westonruter westonruter force-pushed the update/server-side-block-location branch from b1b056d to bfe31c2 Compare July 28, 2017 22:34
@westonruter westonruter removed the [Status] In Progress Tracking issues with work in progress label Jul 28, 2017
@westonruter westonruter requested a review from nylen July 28, 2017 22:35
@westonruter
Copy link
Member Author

@aduth thanks for that. Fixed in bfe31c2

@codecov
Copy link

codecov bot commented Jul 28, 2017

Codecov Report

Merging #2014 into master will increase coverage by 0.16%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2014      +/-   ##
==========================================
+ Coverage   20.21%   20.37%   +0.16%     
==========================================
  Files         135      135              
  Lines        4225     4490     +265     
  Branches      717      802      +85     
==========================================
+ Hits          854      915      +61     
- Misses       2842     2970     +128     
- Partials      529      605      +76
Impacted Files Coverage Δ
blocks/block-alignment-toolbar/index.js 30% <0%> (-10%) ⬇️
blocks/library/text/index.js 34.48% <0%> (-6.7%) ⬇️
editor/state.js 92.3% <0%> (-0.31%) ⬇️
components/icon-button/index.js 100% <0%> (ø) ⬆️
components/dropdown-menu/index.js 0% <0%> (ø) ⬆️
blocks/url-input/index.js 0% <0%> (ø) ⬆️
editor/sidebar/block-inspector/class-name.js 0% <0%> (ø) ⬆️
editor/index.js 0% <0%> (ø) ⬆️
blocks/api/serializer.js 100% <0%> (ø) ⬆️
editor/modes/visual-editor/block.js 0% <0%> (ø) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6c3a4f...b434f25. Read the comment docs.

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Verified package plugin loads block correctly.
Verified composer lint surfaces issues in the block PHP file.

gutenberg.php Outdated
@@ -25,6 +25,7 @@
require_once dirname( __FILE__ ) . '/lib/parser.php';
require_once dirname( __FILE__ ) . '/lib/register.php';

// Register server-side code for individual blocks.
Copy link
Member

Choose a reason for hiding this comment

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

Should we preserve this comment?

@westonruter westonruter merged commit 00d16a6 into master Jul 31, 2017
@nylen nylen deleted the update/server-side-block-location branch July 31, 2017 20:24
Tug pushed a commit that referenced this pull request Mar 19, 2020
…new-block-indicator-style

Update 'Add Block Here' indicator style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants