-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Recent blocks #1694
Recent blocks #1694
Changes from all commits
8f00652
ead5309
e455346
9286b68
1ef4308
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,11 @@ import { combineReducers, applyMiddleware, createStore } from 'redux'; | |
import refx from 'refx'; | ||
import { reduce, keyBy, first, last, omit, without, flowRight } from 'lodash'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { getBlockTypes } from 'blocks'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be internal dependency. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you clarify this? Imports from 'blocks' are WordPress dependencies in other places. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've just checked, and everywhere else that imports things from 'blocks', it's listed as a WordPress dependency. Does everywhere else need to change, or is this specific to the getBlockTypes function? If so, can you clarify why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, sorry, my bad, this is assuming how we'll expose these. We would need to fix this one instead: https://github.com/WordPress/gutenberg/pull/1694/files#diff-d9e543404329206d6652e45f824ae35fR11 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed! |
||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
|
@@ -219,6 +224,28 @@ export const editor = combineUndoableReducers( { | |
|
||
return state; | ||
}, | ||
|
||
recentlyUsedBlocks( state = [], action ) { | ||
const maxRecent = 8; | ||
switch ( action.type ) { | ||
case 'SETUP_NEW_POST': | ||
// This is where we initially populate the recently used blocks, | ||
// for now this inserts blocks from the common category. | ||
return getBlockTypes() | ||
.filter( ( blockType ) => 'common' === blockType.category ) | ||
.slice( 0, maxRecent ) | ||
.map( ( blockType ) => blockType.name ); | ||
case 'INSERT_BLOCKS': | ||
// This is where we record the block usage so it can show up in | ||
// the recent blocks. | ||
let newState = [ ...state ]; | ||
action.blocks.forEach( ( block ) => { | ||
newState = [ block.name, ...without( newState, block.name ) ]; | ||
} ); | ||
return newState.slice( 0, maxRecent ); | ||
} | ||
return state; | ||
}, | ||
}, { resetTypes: [ 'RESET_BLOCKS' ] } ); | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Add" feels a bit weird in this context. Why does it need to "add" anything? Should recent be a "category" or just returned on demand for the recent tab?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it's "adding" .recent to the object that has the blocks groups by category. We'd need to rework the render method a bit to get rid of this, which I'm happy to do, it's just this PR was getting big enough as it was :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify why this needs a separate step, 'recent' isn't a category because we don't have the ability for a block to be in two categories, so the method that sorts blocks by category has no knowledge of 'recent', and this method is adding the recent category to the object that holds the blocks grouped by category.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, the render method could use cleaning up, there's a lot of duplication and we could simplify it a lot. Your call if you'd like this done in this PR or not, I'm happy to do it either here, or as another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am ok refactoring later.