-
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
Show most frequently used blocks next to inserter #2877
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2877 +/- ##
=========================================
- Coverage 34.06% 33.87% -0.2%
=========================================
Files 192 193 +1
Lines 5677 5789 +112
Branches 997 1012 +15
=========================================
+ Hits 1934 1961 +27
- Misses 3166 3237 +71
- Partials 577 591 +14
Continue to review full report at Codecov.
|
@@ -26,11 +26,17 @@ describe( 'VisualEditorInserter', () => { | |||
expect( wrapper.state( 'isShowingControls' ) ).toBe( false ); | |||
} ); | |||
|
|||
it( 'should insert paragraph block', () => { | |||
/* it( 'should insert paragraph block', () => { | |||
const onInsertBlock = jest.fn(); | |||
const wrapper = shallow( | |||
<VisualEditorInserter onInsertBlock={ onInsertBlock } /> |
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.
Actually we do not test the "connected" component but just the presentational one, so you just need to provide "mostFrequentlyUsedBlocks" as a prop.
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.
This is looking very good for me.
I think the getMostFrequentlyUsedBlocks method needs to be memoized
Sounds like a good idea 👍
Memoize added, and tests fixed up for the inserter. |
editor/selectors.js
Outdated
* | ||
* @param {Object} state Global application state | ||
* @return {Array} List of block type settings | ||
*/ | ||
export function getMostFrequentlyUsedBlocks( state ) { | ||
export const getMostFrequentlyUsedBlocks = memoize( ( state ) => { |
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.
Should we use createSelector
helper instead to memoize the selector (we already use it elsewhere). https://github.com/aduth/rememo
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 was going to suggest something like this, I didn't see we already had something :) Will fix!
While I don't like hardcoding, I wonder if we should hard code the paragraph and image block as default if no usage yet (try clearing localstorage). Also flagging this for design review, since it changes the quick blocks behavior. cc @jasmussen |
Yeah I guess that's easy enough to do in the selector. |
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.
⭐️
* master: (45 commits) Parser: Hide the core namespace in serialised data (WordPress#2950) fix: Undefined index warning (WordPress#2982) navigation via keydown PHPCS improvements (WordPress#2914) Update/html block description (WordPress#2917) Framework: Merge EditorSettingsProvider into EditorProvider Framework: Move the store initialization to a dedicated component Rotate header ellipsis to match block ellipsis add/459: Added support for ins, sub and sup Refresh nonce with heartbeat (WordPress#2790) Paste: add table support Bump version to 1.4.0. (WordPress#2954) Blocks: The custom classname should be persisted properly in the block's comment Editor: Fix the scroll position when reordering blocks Polish spacing in block ellipsis menu (WordPress#2955) Editor: HTML Editor per block (WordPress#2797) Show most frequently used blocks next to inserter (WordPress#2877) add/459: Added light grey background to selected inline boundaries Extend inline boundaries to other elements Move markdown fix to own plugin-compatibility file ...
* master: (45 commits) Parser: Hide the core namespace in serialised data (WordPress#2950) fix: Undefined index warning (WordPress#2982) navigation via keydown PHPCS improvements (WordPress#2914) Update/html block description (WordPress#2917) Framework: Merge EditorSettingsProvider into EditorProvider Framework: Move the store initialization to a dedicated component Rotate header ellipsis to match block ellipsis add/459: Added support for ins, sub and sup Refresh nonce with heartbeat (WordPress#2790) Paste: add table support Bump version to 1.4.0. (WordPress#2954) Blocks: The custom classname should be persisted properly in the block's comment Editor: Fix the scroll position when reordering blocks Polish spacing in block ellipsis menu (WordPress#2955) Editor: HTML Editor per block (WordPress#2797) Show most frequently used blocks next to inserter (WordPress#2877) add/459: Added light grey background to selected inline boundaries Extend inline boundaries to other elements Move markdown fix to own plugin-compatibility file ...
* master: (45 commits) Parser: Hide the core namespace in serialised data (WordPress#2950) fix: Undefined index warning (WordPress#2982) navigation via keydown PHPCS improvements (WordPress#2914) Update/html block description (WordPress#2917) Framework: Merge EditorSettingsProvider into EditorProvider Framework: Move the store initialization to a dedicated component Rotate header ellipsis to match block ellipsis add/459: Added support for ins, sub and sup Refresh nonce with heartbeat (WordPress#2790) Paste: add table support Bump version to 1.4.0. (WordPress#2954) Blocks: The custom classname should be persisted properly in the block's comment Editor: Fix the scroll position when reordering blocks Polish spacing in block ellipsis menu (WordPress#2955) Editor: HTML Editor per block (WordPress#2797) Show most frequently used blocks next to inserter (WordPress#2877) add/459: Added light grey background to selected inline boundaries Extend inline boundaries to other elements Move markdown fix to own plugin-compatibility file ...
There's something disconcerting to me about the dynamic nature of this, that it's less predictable. Maybe I'm attached to the idea that there's some comfort in knowing I'd always find the same options in this control, an especially important comfort considering that the controls are hidden until revealed by hover. Maybe I'm overthinking it 😄 |
Initially you'll notice changes, but my thinking was that over time, it'll stabilise, and at least he first two options will be pretty much static, but tailored for the user's usage. |
Description
Instead of hardcoded paragraph and image blocks, this shows
the three most frequently used blocks, based on stored blockUsage
The algorithm for working out the blocks to display is just "most frequently used." I think that's fine for now, but we probably want to think about future refinements that "age" the usage too, in the future.
How to test
Hover over the inserter icon in the main editing section, and your three most used blocks should appear next to it. Click on them to insert them.
Clear your local storage, and paragraph and image should be displayed to start with.