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.

Sign up for GitHub

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

Jump to bottom

Try: use 'frecency' to sort items in the inserters #5342

Merged
merged 2 commits into from
Mar 5, 2018
Merged

Try: use 'frecency' to sort items in the inserters #5342

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4632d01
Use 'frecency' to sort items in the inserters
noisysocks Mar 2, 2018
f754b1a
Rename 'Recent' tab to 'Frequent'
noisysocks Mar 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions editor/components/inserter-with-shortcuts/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { __, sprintf } from '@wordpress/i18n';
*/
import './style.scss';
import Inserter from '../inserter';
import { getFrequentInserterItems } from '../../store/selectors';
import { getFrecentInserterItems } from '../../store/selectors';
import { replaceBlocks } from '../../store/actions';

function InserterWithShortcuts( { items, isLocked, onToggle, onInsert } ) {
Expand Down Expand Up @@ -62,7 +62,7 @@ export default compose(
} ),
connect(
( state, { enabledBlockTypes } ) => ( {
items: getFrequentInserterItems( state, enabledBlockTypes, 3 ),
items: getFrecentInserterItems( state, enabledBlockTypes, 3 ),
} ),
( dispatch, { uid, layout } ) => ( {
onInsert( { name, initialAttributes } ) {
Expand Down
8 changes: 4 additions & 4 deletions editor/components/inserter/menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { keycodes } from '@wordpress/utils';
import './style.scss';
import NoBlocks from './no-blocks';

import { getInserterItems, getRecentInserterItems } from '../../store/selectors';
import { getInserterItems, getFrecentInserterItems } from '../../store/selectors';
import { fetchReusableBlocks } from '../../store/actions';
import { default as InserterGroup } from './group';
import BlockPreview from '../block-preview';
Expand Down Expand Up @@ -118,7 +118,7 @@ export class InserterMenu extends Component {
}

getItemsForTab( tab ) {
const { items, recentItems } = this.props;
const { items, frecentItems } = this.props;

// If we're searching, use everything, otherwise just get the items visible in this tab
if ( this.state.filterValue ) {
Expand All @@ -128,7 +128,7 @@ export class InserterMenu extends Component {
let predicate;
switch ( tab ) {
case 'recent':
return recentItems;
return frecentItems;

case 'blocks':
predicate = ( item ) => item.category !== 'embed' && item.category !== 'reusable-blocks';
Expand Down Expand Up @@ -344,7 +344,7 @@ export default compose(
( state, ownProps ) => {
return {
items: getInserterItems( state, ownProps.enabledBlockTypes ),
recentItems: getRecentInserterItems( state, ownProps.enabledBlockTypes ),
frecentItems: getFrecentInserterItems( state, ownProps.enabledBlockTypes ),
};
},
{ fetchReusableBlocks }
Expand Down
18 changes: 9 additions & 9 deletions editor/components/inserter/test/menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe( 'InserterMenu', () => {
position={ 'top center' }
instanceId={ 1 }
items={ [] }
recentItems={ [] }
frecentItems={ [] }
debouncedSpeak={ noop }
fetchReusableBlocks={ noop }
blockTypes
Expand All @@ -114,7 +114,7 @@ describe( 'InserterMenu', () => {
position={ 'top center' }
instanceId={ 1 }
items={ [] }
recentItems={ [] }
frecentItems={ [] }
debouncedSpeak={ noop }
fetchReusableBlocks={ noop }
/>
Expand All @@ -130,7 +130,7 @@ describe( 'InserterMenu', () => {
position={ 'top center' }
instanceId={ 1 }
items={ items }
recentItems={ [ advancedTextItem, textItem, someOtherItem ] }
frecentItems={ [ advancedTextItem, textItem, someOtherItem ] }
debouncedSpeak={ noop }
fetchReusableBlocks={ noop }
/>
Expand All @@ -149,7 +149,7 @@ describe( 'InserterMenu', () => {
position={ 'top center' }
instanceId={ 1 }
items={ items }
recentItems={ [] }
frecentItems={ [] }
debouncedSpeak={ noop }
fetchReusableBlocks={ noop }
/>
Expand All @@ -173,7 +173,7 @@ describe( 'InserterMenu', () => {
position={ 'top center' }
instanceId={ 1 }
items={ items }
recentItems={ [] }
frecentItems={ [] }
debouncedSpeak={ noop }
fetchReusableBlocks={ noop }
/>
Expand All @@ -196,7 +196,7 @@ describe( 'InserterMenu', () => {
position={ 'top center' }
instanceId={ 1 }
items={ items }
recentItems={ [] }
frecentItems={ [] }
debouncedSpeak={ noop }
fetchReusableBlocks={ noop }
/>
Expand All @@ -222,7 +222,7 @@ describe( 'InserterMenu', () => {
position={ 'top center' }
instanceId={ 1 }
items={ items }
recentItems={ items }
frecentItems={ items }
debouncedSpeak={ noop }
fetchReusableBlocks={ noop }
/>
Expand All @@ -239,7 +239,7 @@ describe( 'InserterMenu', () => {
position={ 'top center' }
instanceId={ 1 }
items={ items }
recentItems={ [] }
frecentItems={ [] }
debouncedSpeak={ noop }
fetchReusableBlocks={ noop }
/>
Expand All @@ -262,7 +262,7 @@ describe( 'InserterMenu', () => {
position={ 'top center' }
instanceId={ 1 }
items={ items }
recentItems={ [] }
frecentItems={ [] }
debouncedSpeak={ noop }
fetchReusableBlocks={ noop }
/>
Expand Down
1 change: 0 additions & 1 deletion editor/store/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
export const PREFERENCES_DEFAULTS = {
recentInserts: [],
insertUsage: {},
};
8 changes: 1 addition & 7 deletions editor/store/reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -656,17 +656,12 @@ export function preferences( state = PREFERENCES_DEFAULTS, action ) {
id += '/' + block.attributes.ref;
}

const isSameAsInsert = ( { name, ref } ) => name === insert.name && ref === insert.ref;

return {
...prevState,
recentInserts: [
insert,
...reject( prevState.recentInserts, isSameAsInsert ),
],
insertUsage: {
...prevState.insertUsage,
[ id ]: {
time: Date.now(),
Copy link
Member

@aduth aduth Mar 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Pedantry: Date.now makes this reducer impure:

https://en.wikipedia.org/wiki/Functional_programming#Pure_functions

If a pure function is called with arguments that cause no side-effects, the result is constant with respect to that argument list ... i.e., if calling the pure function again with the same arguments returns the same result.

Violating a core principle of Redux:

Reducers are just pure functions that take the previous state and an action, and return the next state.

https://redux.js.org/introduction/three-principles#changes-are-made-with-pure-functions

Copy link
Member Author

Choose a reason for hiding this comment

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

You're totally right. I'll move this to the action creator.

count: prevState.insertUsage[ id ] ? prevState.insertUsage[ id ].count + 1 : 1,
insert,
},
Expand All @@ -678,7 +673,6 @@ export function preferences( state = PREFERENCES_DEFAULTS, action ) {
return {
...state,
insertUsage: omitBy( state.insertUsage, ( { insert } ) => insert.ref === action.id ),
recentInserts: reject( state.recentInserts, insert => insert.ref === action.id ),
};
}

Expand Down
35 changes: 20 additions & 15 deletions editor/store/selectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1186,30 +1186,35 @@ function getItemsFromInserts( state, inserts, enabledBlockTypes = true, maximum
}

/**
* Determines the items that appear in the 'Recent' tab of the inserter.
* Returns a list of items which the user is likely to want to insert. These
* are ordered by 'frecency', which is a heuristic that combines block usage
* frequency and recency.
*
* @param {Object} state Global application state.
* @param {string[]|boolean} enabledBlockTypes Enabled block types, or true/false to enable/disable all types.
* @param {number} maximum Number of items to return.
*
* @return {Editor.InserterItem[]} Items that appear in the 'Recent' tab.
*/
export function getRecentInserterItems( state, enabledBlockTypes = true, maximum = MAX_RECENT_BLOCKS ) {
return getItemsFromInserts( state, state.preferences.recentInserts, enabledBlockTypes, maximum );
}

/**
* Determines the items that appear in the inserter with shortcuts based on the block usage
* https://en.wikipedia.org/wiki/Frecency
*
* @param {Object} state Global application state.
* @param {string[]|boolean} enabledBlockTypes Enabled block types, or true/false to enable/disable all types.
* @param {number} maximum Number of items to return.
*
* @return {Editor.InserterItem[]} Items that appear in the 'Recent' tab.
*/
export function getFrequentInserterItems( state, enabledBlockTypes = true, maximum = MAX_RECENT_BLOCKS ) {
export function getFrecentInserterItems( state, enabledBlockTypes = true, maximum = MAX_RECENT_BLOCKS ) {
const calculateFrecency = ( time, count ) => {
const duration = Date.now() - time;
switch ( true ) {
case duration < 3600:
return count * 4;
case duration < ( 24 * 3600 ):
return count * 2;
case duration < ( 7 * 24 * 3600 ):
return count / 2;
default:
return count / 4;
}
};

const sortedInserts = values( state.preferences.insertUsage )
.sort( ( a, b ) => b.count - a.count )
.sort( ( a, b ) => calculateFrecency( b.time, b.count ) - calculateFrecency( a.time, a.count ) )
.map( ( { insert } ) => insert );
return getItemsFromInserts( state, sortedInserts, enabledBlockTypes, maximum );
}
Expand Down
25 changes: 6 additions & 19 deletions editor/store/test/reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1196,13 +1196,12 @@ describe( 'state', () => {
const state = preferences( undefined, {} );

expect( state ).toEqual( {
recentInserts: [],
insertUsage: {},
} );
} );

it( 'should record recently used blocks', () => {
const state = preferences( deepFreeze( { recentInserts: [], insertUsage: {} } ), {
const state = preferences( deepFreeze( { insertUsage: {} } ), {
type: 'INSERT_BLOCKS',
blocks: [ {
uid: 'bacon',
Expand All @@ -1211,21 +1210,19 @@ describe( 'state', () => {
} );

expect( state ).toEqual( {
recentInserts: [
{ name: 'core-embed/twitter' },
],
insertUsage: {
'core-embed/twitter': {
time: expect.any( Number ),
Copy link
Member

Choose a reason for hiding this comment

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

To the above point, we shouldn't have any need for expect.any in this file, given that a reducer should be deterministic.

count: 1,
insert: { name: 'core-embed/twitter' },
},
},
} );

const twoRecentBlocks = preferences( deepFreeze( {
recentInserts: [],
insertUsage: {
'core-embed/twitter': {
time: expect.any( Number ),
count: 1,
insert: { name: 'core-embed/twitter' },
},
Expand All @@ -1243,16 +1240,14 @@ describe( 'state', () => {
} );

expect( twoRecentBlocks ).toEqual( {
recentInserts: [
{ name: 'core/block', ref: 123 },
{ name: 'core-embed/twitter' },
],
insertUsage: {
'core-embed/twitter': {
time: expect.any( Number ),
count: 2,
insert: { name: 'core-embed/twitter' },
},
'core/block/123': {
time: expect.any( Number ),
count: 1,
insert: { name: 'core/block', ref: 123 },
},
Expand All @@ -1262,13 +1257,9 @@ describe( 'state', () => {

it( 'should remove recorded reusable blocks that are deleted', () => {
const initialState = {
recentInserts: [
{ name: 'core-embed/twitter' },
{ name: 'core/block', ref: 123 },
{ name: 'core/block', ref: 456 },
],
insertUsage: {
'core/block/123': {
time: 1000,
count: 1,
insert: { name: 'core/block', ref: 123 },
},
Expand All @@ -1281,10 +1272,6 @@ describe( 'state', () => {
} );

expect( state ).toEqual( {
recentInserts: [
{ name: 'core-embed/twitter' },
{ name: 'core/block', ref: 456 },
],
insertUsage: {},
} );
} );
Expand Down
Loading