Add reusable blocks data layer #3017

noisysocks · 2017-10-13T07:19:30Z

Description

Adds the actions, selectors and reducers necessary for supporting reusable blocks. There are 5 actions:

FETCH_REUSABLE_BLOCKS—Loads reusable blocks from the API and inserts them into the store.
UPDATE_REUSABLE_BLOCK—Inserts and updates a reusable block that is in the store.
SAVE_REUSABLE_BLOCK—Persists a reusable block that's in the store to the API.
MAKE_BLOCK_STATIC—Transforms a reusable block on the page into a regular block.
MAKE_BLOCK_REUSABLE—Transforms a regular block on the page into a reusable block.

#1516 describes the feature and #2081 (#2081 (comment), in particular) describes the technical approach I'm taking.

This PR is a subset of #2659, which was too large. Check it out though if you'd like to play with the complete feature!

How Has This Been Tested?

Unit tests are included for the new actions, selectors and reducers.

I changed the inserter to ignore blocks that are marked with isPrivate: true, so it's worth testing the inserter to make sure that there are no regressions.

cc. @mtias @aduth @youknowriad

❤️

youknowriad

Left some small comments, but this is in a good shape. Nice work

youknowriad · 2017-10-13T08:54:43Z

editor/actions.js

+ * @param {string} id If given, only a single reusable block with this ID will be fetched
+ * @return {Object}   Action object
+ */
+export function fetchReusableBlocks( id = null ) {


Minor: do we really need the default value?

I suppose not! Removed in 7762cb73a84b08f60938bc004cfd1fa20bccf36a.

youknowriad · 2017-10-13T08:58:04Z

editor/reducer.js

+				...newState,
+				[ reusableBlock.id ]: {
+					...reusableBlock,
+					isSaving: false,


Why are we merging the isSaving flag with the reusable block object? My first reaction here is: "Should this be in a separate reducer (or subReducer)?

No reason other than that it's convenient to pass the block's data and metadata (isSaving) to UI components all in one go.

I'd be happy to move this to a different reducer. Perhaps the shape of the redux tree could be something like:

reusableBlocks: { data: { 'f5ca58b0-aad0-4777-bb10-f3d0fa08a163': /* the block */, }, isSaving: { 'f5ca58b0-aad0-4777-bb10-f3d0fa08a163': false, }, }

Perhaps the shape of the redux tree could be something like

I like this proposal. Request flags could be generic, it makes sense to me to separate them from the data.

👌 done in fb9d4c70a898908eddcf9b71028d16262d171791.

youknowriad

Nice work 👍

youknowriad · 2017-10-18T08:24:29Z

editor/reducer.js

+				return reduce( action.reusableBlocks, ( newState, reusableBlock ) => ( {
+					...newState,
+					[ reusableBlock.id ]: {
+						...reusableBlock,


Is there a reason we're still cloning the object?

youknowriad · 2017-10-18T08:26:14Z

editor/reducer.js

-			const { id } = action;
+	isSaving( state = {}, action ) {
+		switch ( action.type ) {
+			case 'SAVE_REUSABLE_BLOCK': {


I guess the scoping ({) is not useful anymore

mtias · 2017-10-25T14:36:15Z

blocks/api/categories.js

@@ -17,6 +17,7 @@ const categories = [
 	{ slug: 'layout', title: __( 'Layout Blocks' ) },
 	{ slug: 'widgets', title: __( 'Widgets' ) },
 	{ slug: 'embed', title: __( 'Embed' ) },
+	{ slug: 'reusable-blocks', title: __( 'My Reusable Blocks' ) },


These should probably be allocated in another "tab" (like recent, blocks, embeds) rather than a category. The names should probably be "Saved". cc @jasmussen

Adds the actions, selectors and reducers necessary for supporting reusable blocks. There are 5 actions: - FETCH_REUSABLE_BLOCKS: Loads reusable blocks from the API and inserts them into the store. - UPDATE_REUSABLE_BLOCK: Inserts of updates a reusable block that is in the store. - SAVE_REUSABLE_BLOCK: Persists a reusable block that's in the store to the API. - MAKE_BLOCK_STATIC: Transforms a reusable block on the page into a regular block. - MAKE_BLOCK_REUSABLE: Transforms a regular block on the page into a reusable block.

Re-shape the Redux state tree so that metadata like `isSaving` is kept seperately from each reusable block's actual data.

codecov · 2017-11-07T09:37:53Z

Codecov Report

Merging #3017 into master will increase coverage by 0.24%.
The diff coverage is 96.55%.

@@            Coverage Diff            @@
##           master   #3017      +/-   ##
=========================================
+ Coverage   31.06%   31.3%   +0.24%     
=========================================
  Files         245     246       +1     
  Lines        6716    6741      +25     
  Branches     1205    1209       +4     
=========================================
+ Hits         2086    2110      +24     
- Misses       3895    3896       +1     
  Partials      735     735

Impacted Files	Coverage Δ
blocks/api/categories.js	`100% <ø> (ø)`	⬆️
editor/reducer.js	`90.76% <100%> (+0.64%)`	⬆️
editor/inserter/menu.js	`49.6% <100%> (+0.4%)`	⬆️
editor/actions.js	`44.23% <100%> (+5.93%)`	⬆️
editor/selectors.js	`93.47% <100%> (+0.1%)`	⬆️
blocks/api/factory.js	`90.62% <100%> (+0.3%)`	⬆️
blocks/library/reusable-block/index.js	`66.66% <66.66%> (ø)`

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 e7bdfee...d6e0cf2. Read the comment docs.

aduth · 2018-01-29T10:48:23Z

blocks/api/factory.js

+ */
+export function createReusableBlock( type, attributes ) {
+	return {
+		id: uuid(),


The apparent overlap yet disconnect between a reusable block's id property and a standard block's uid property is unfortunate. I can see why we might want to call this one id, since it aligns well with how it is modeled on the server (the post's ID), but it makes me wonder whether we should then align the standard block property to id as well. I'm not strongly attached to uid as a property name.

Do you have any thoughts?

I'd be into aligning them so long as we're explicit when writing code that interacts with both types of ID, e.g:

const blockId = block.id; const reusableBlockId = block.attributes.ref;

This would be a breaking change though, since there might be third party code that references block.uid.

aduth · 2018-01-29T10:50:47Z

blocks/api/factory.js

+export function createReusableBlock( type, attributes ) {
+	return {
+		id: uuid(),
+		name: __( 'Untitled block' ),


Would there ever be a case where we want to create a reusable block and assign its name immediately? A bit awkward to achieve now, since name is not accepted as an argument of the function.

No such case was in our design so I figured you aren't gonna need it. We could add an optional argument in the future if it comes up.

I think we should support this and have UI that focus on the name input before saving. cc @jasmussen

noisysocks mentioned this pull request Oct 13, 2017

Add reusable blocks #2659

Closed

youknowriad reviewed Oct 13, 2017

View reviewed changes

youknowriad approved these changes Oct 18, 2017

View reviewed changes

mtias reviewed Oct 25, 2017

View reviewed changes

noisysocks added 3 commits November 7, 2017 10:36

Remove default param in lieu of documenting the param as optional

19c86aa

Seperate metadata (isSaving) from reusable block data in the Redux tree

ae8fc05

Re-shape the Redux state tree so that metadata like `isSaving` is kept seperately from each reusable block's actual data.

Reusable Block Reducer: small tweaks

d6e0cf2

youknowriad merged commit fa59bf0 into WordPress:master Nov 7, 2017

aduth added the [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) label Dec 19, 2017

aduth reviewed Jan 29, 2018

View reviewed changes

aduth mentioned this pull request Jan 29, 2018

Effects: Use block API functions in reusable effects #4732

Merged

noisysocks deleted the add/reusable-blocks-data-layer branch January 29, 2018 23:22

noisysocks mentioned this pull request Feb 7, 2018

Reusable block name auto save #4527

Closed

noisysocks mentioned this pull request Jun 4, 2018

Shared Blocks: Refactor fetching/saving/updating to avoid cycle dependency #7080

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add reusable blocks data layer #3017

Add reusable blocks data layer #3017

noisysocks commented Oct 13, 2017 •

edited

Loading

youknowriad left a comment

youknowriad Oct 13, 2017

noisysocks Oct 17, 2017

youknowriad Oct 13, 2017 •

edited

Loading

noisysocks Oct 17, 2017

youknowriad Oct 17, 2017

noisysocks Oct 17, 2017

youknowriad left a comment

youknowriad Oct 18, 2017

youknowriad Oct 18, 2017

mtias Oct 25, 2017

codecov bot commented Nov 7, 2017 •

edited

Loading

aduth Jan 29, 2018 •

edited

Loading

noisysocks Jan 29, 2018

aduth Jan 29, 2018 •

edited

Loading

noisysocks Jan 29, 2018

mtias Feb 7, 2018

Add reusable blocks data layer #3017

Add reusable blocks data layer #3017

Conversation

noisysocks commented Oct 13, 2017 • edited Loading

Description

How Has This Been Tested?

youknowriad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad Oct 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 7, 2017 • edited Loading

Codecov Report

aduth Jan 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth Jan 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noisysocks commented Oct 13, 2017 •

edited

Loading

youknowriad Oct 13, 2017 •

edited

Loading

codecov bot commented Nov 7, 2017 •

edited

Loading

aduth Jan 29, 2018 •

edited

Loading

aduth Jan 29, 2018 •

edited

Loading