From 441b0de359d85414ec941fee2d3d885fb144365b Mon Sep 17 00:00:00 2001 From: James Nylen Date: Tue, 1 Aug 2017 17:55:55 -0400 Subject: [PATCH 01/11] Add mechanism for opt-in usage tracking --- components/button/index.js | 17 ++++++- components/notice/style.scss | 2 +- editor/actions.js | 1 + editor/enable-tracking-prompt/index.js | 56 ++++++++++++++++++++++ editor/enable-tracking-prompt/style.scss | 21 ++++++++ editor/index.js | 10 +++- editor/utils/tracking.js | 61 ++++++++++++++++++++++++ 7 files changed, 164 insertions(+), 4 deletions(-) create mode 100644 editor/enable-tracking-prompt/index.js create mode 100644 editor/enable-tracking-prompt/style.scss create mode 100644 editor/utils/tracking.js diff --git a/components/button/index.js b/components/button/index.js index dde66367f378d8..dda515022caba1 100644 --- a/components/button/index.js +++ b/components/button/index.js @@ -26,11 +26,24 @@ class Button extends Component { } render() { - const { href, target, isPrimary, isLarge, isToggled, className, disabled, ...additionalProps } = this.props; + const { + href, + target, + isPrimary, + isSecondary, + isLarge, + isSmall, + isToggled, + className, + disabled, + ...additionalProps, // eslint-disable-line comma-dangle + } = this.props; const classes = classnames( 'components-button', className, { - button: ( isPrimary || isLarge ), + button: ( isPrimary || isSecondary || isLarge ), 'button-primary': isPrimary, + 'button-secondary': isSecondary, 'button-large': isLarge, + 'button-small': isSmall, 'is-toggled': isToggled, } ); diff --git a/components/notice/style.scss b/components/notice/style.scss index a57109491d4683..eb91ea840d49ee 100644 --- a/components/notice/style.scss +++ b/components/notice/style.scss @@ -3,6 +3,6 @@ top: 40px; right: 0; min-width: 300px; - max-width: 400px; + max-width: 650px; z-index: z-index( ".components-notice-list" ); } diff --git a/editor/actions.js b/editor/actions.js index 73ca75e43f3565..a31046fc9592c6 100644 --- a/editor/actions.js +++ b/editor/actions.js @@ -234,5 +234,6 @@ export function removeNotice( id ) { } export const createSuccessNotice = partial( createNotice, 'success' ); +export const createInfoNotice = partial( createNotice, 'info' ); export const createErrorNotice = partial( createNotice, 'error' ); export const createWarningNotice = partial( createNotice, 'warning' ); diff --git a/editor/enable-tracking-prompt/index.js b/editor/enable-tracking-prompt/index.js new file mode 100644 index 00000000000000..9aabc436dcd65e --- /dev/null +++ b/editor/enable-tracking-prompt/index.js @@ -0,0 +1,56 @@ +/** + * External dependencies + */ +import { connect } from 'react-redux'; + +/** + * WordPress dependencies + */ +import { __ } from 'i18n'; +import { Button } from 'components'; + +/** + * Internal dependencies + */ +import './style.scss'; +import { removeNotice } from '../actions'; + +function EnableTrackingPrompt( { dispatchRemoveNotice } ) { + function dismissTrackingPrompt( result ) { + window.setUserSetting( 'gutenberg_tracking', result ); + dispatchRemoveNotice( 'notice:enable-tracking-prompt' ); + } + + return ( +
+
+ { __( 'Can Gutenberg collect data about your usage of the editor?' ) } +
+ + +
+
+
+ { __( 'Usage data is completely anonymous and will only be used to improve the editor.' ) } +
+
+ ); +} + +export default connect( + undefined, + { dispatchRemoveNotice: removeNotice } +)( EnableTrackingPrompt ); + diff --git a/editor/enable-tracking-prompt/style.scss b/editor/enable-tracking-prompt/style.scss new file mode 100644 index 00000000000000..ed7d7f0382ac61 --- /dev/null +++ b/editor/enable-tracking-prompt/style.scss @@ -0,0 +1,21 @@ +.enable-tracking-prompt { + .enable-tracking-prompt__message { + font-weight: bold; + margin-top: 8px; + + .enable-tracking-prompt__buttons { + float: right; + + .button { + margin-left: 6px; + } + } + } + + .enable-tracking-prompt__clarification { + clear: both; + margin-bottom: 8px; + font-size: 90%; + font-style: italic; + } +} diff --git a/editor/index.js b/editor/index.js index 9a17f6ab72b8f0..625525441a3ef7 100644 --- a/editor/index.js +++ b/editor/index.js @@ -20,7 +20,8 @@ import { settings } from '@wordpress/date'; import './assets/stylesheets/main.scss'; import Layout from './layout'; import { createReduxStore } from './state'; -import { undo } from './actions'; +import { undo, createInfoNotice } from './actions'; +import EnableTrackingPrompt from './enable-tracking-prompt'; import EditorSettingsProvider from './settings/provider'; /** @@ -98,6 +99,13 @@ export function createEditorInstance( id, post, editorSettings = DEFAULT_SETTING settings: editorSettings, } ); + if ( window.getUserSetting( 'gutenberg_tracking' ) === '' ) { + store.dispatch( createInfoNotice( + , + 'notice:enable-tracking-prompt' + ) ); + } + preparePostState( store, post ); render( diff --git a/editor/utils/tracking.js b/editor/utils/tracking.js new file mode 100644 index 00000000000000..3d8faca789861c --- /dev/null +++ b/editor/utils/tracking.js @@ -0,0 +1,61 @@ +/* eslint-disable no-console */ + +export function bumpStat( group, name ) { + if ( typeof group !== 'string' || typeof name !== 'string' ) { + console.error( + 'Stat group names and stat names must be strings.' + ); + return; + } + + if ( /[^a-z0-9_]/.test( group ) ) { + console.error( + 'Stat group names must consist of letters, numbers, and underscores.' + ); + return; + } + + if ( group.length > 22 ) { // 32 - 'gutenberg_'.length + console.error( + 'Stat group names cannot be longer than 22 characters.' + ); + return; + } + + if ( /[^a-z0-9_-]/.test( name ) ) { + console.error( + 'Stat names must consist of letters, numbers, underscores, and dashes.' + ); + return; + } + + if ( name.length > 32 ) { + console.error( + 'Stat names cannot be longer than 32 characters.' + ); + return; + } + + if ( window.getUserSetting( 'gutenberg_tracking' ) !== 'on' ) { + return; + } + + const src = document.location.protocol + + '//pixel.wp.com/g.gif?v=wpcom-no-pv' + + '&x_gutenberg_' + encodeURIComponent( group ) + + '=' + encodeURIComponent( name ) + + '&t=' + Math.random(); + + if ( process.env.NODE_ENV === 'development' ) { + console.log( + 'Skipping stats collection for non-production build:', + src + ); + } + + if ( process.env.NODE_ENV !== 'production' ) { + return src; + } + + new window.Image().src = src; +} From 2cab1faad7e4977c9c5021fe006fa7ec1234cb24 Mon Sep 17 00:00:00 2001 From: James Nylen Date: Tue, 1 Aug 2017 18:10:56 -0400 Subject: [PATCH 02/11] Track which blocks are added to post content --- editor/inserter/index.js | 3 +++ editor/modes/visual-editor/block-list.js | 3 +++ 2 files changed, 6 insertions(+) diff --git a/editor/inserter/index.js b/editor/inserter/index.js index 1f210bdfc8964e..aee767446526d2 100644 --- a/editor/inserter/index.js +++ b/editor/inserter/index.js @@ -18,6 +18,7 @@ import { createBlock } from '@wordpress/blocks'; import InserterMenu from './menu'; import { getBlockInsertionPoint, getEditorMode } from '../selectors'; import { insertBlock, hideInsertionPoint } from '../actions'; +import { bumpStat } from '../utils/tracking'; class Inserter extends Component { constructor() { @@ -49,6 +50,8 @@ class Inserter extends Component { name, insertionPoint ); + bumpStat( 'add_block_inserter', name.replace( /\//g, '__' ) ); + bumpStat( 'add_block_total', name.replace( /\//g, '__' ) ); } this.close(); diff --git a/editor/modes/visual-editor/block-list.js b/editor/modes/visual-editor/block-list.js index ec1fca2cee3037..96148bf28cc62a 100644 --- a/editor/modes/visual-editor/block-list.js +++ b/editor/modes/visual-editor/block-list.js @@ -29,6 +29,7 @@ import { getMultiSelectedBlockUids, } from '../../selectors'; import { insertBlock, multiSelect } from '../../actions'; +import { bumpStat } from '../../utils/tracking'; const INSERTION_POINT_PLACEHOLDER = '[[insertion-point]]'; const { ENTER } = keycodes; @@ -201,6 +202,8 @@ class VisualEditorBlockList extends Component { insertBlock( name ) { const newBlock = createBlock( name ); this.props.onInsertBlock( newBlock ); + bumpStat( 'add_block_quick', name.replace( /\//g, '__' ) ); + bumpStat( 'add_block_total', name.replace( /\//g, '__' ) ); } toggleContinueWritingControls( showContinueWritingControls ) { From b7255153a53af56c9dd48fc81600a80694ca4175 Mon Sep 17 00:00:00 2001 From: James Nylen Date: Wed, 2 Aug 2017 13:04:21 -0400 Subject: [PATCH 03/11] Use a different way of avoiding eslint warning --- editor/enable-tracking-prompt/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/editor/enable-tracking-prompt/index.js b/editor/enable-tracking-prompt/index.js index 9aabc436dcd65e..4ff3ab71ceef76 100644 --- a/editor/enable-tracking-prompt/index.js +++ b/editor/enable-tracking-prompt/index.js @@ -15,10 +15,10 @@ import { Button } from 'components'; import './style.scss'; import { removeNotice } from '../actions'; -function EnableTrackingPrompt( { dispatchRemoveNotice } ) { +function EnableTrackingPrompt( props ) { function dismissTrackingPrompt( result ) { window.setUserSetting( 'gutenberg_tracking', result ); - dispatchRemoveNotice( 'notice:enable-tracking-prompt' ); + props.removeNotice( 'notice:enable-tracking-prompt' ); } return ( @@ -51,6 +51,6 @@ function EnableTrackingPrompt( { dispatchRemoveNotice } ) { export default connect( undefined, - { dispatchRemoveNotice: removeNotice } + { removeNotice } )( EnableTrackingPrompt ); From 16b882bbe4013af59e81f7569cbe0308f6087933 Mon Sep 17 00:00:00 2001 From: James Nylen Date: Thu, 3 Aug 2017 09:47:31 -0400 Subject: [PATCH 04/11] Update WordPress dependencies after #2172 --- editor/enable-tracking-prompt/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/editor/enable-tracking-prompt/index.js b/editor/enable-tracking-prompt/index.js index 4ff3ab71ceef76..f7f821c068a4f8 100644 --- a/editor/enable-tracking-prompt/index.js +++ b/editor/enable-tracking-prompt/index.js @@ -6,8 +6,8 @@ import { connect } from 'react-redux'; /** * WordPress dependencies */ -import { __ } from 'i18n'; -import { Button } from 'components'; +import { __ } from '@wordpress/i18n'; +import { Button } from '@wordpress/components'; /** * Internal dependencies From 32d3e3101a81a36ae1e3760b7ecdba0f71b3b814 Mon Sep 17 00:00:00 2001 From: James Nylen Date: Thu, 3 Aug 2017 09:50:14 -0400 Subject: [PATCH 05/11] Update stats URL message --- editor/utils/tracking.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor/utils/tracking.js b/editor/utils/tracking.js index 3d8faca789861c..a8bb4fd5febc2e 100644 --- a/editor/utils/tracking.js +++ b/editor/utils/tracking.js @@ -48,7 +48,7 @@ export function bumpStat( group, name ) { if ( process.env.NODE_ENV === 'development' ) { console.log( - 'Skipping stats collection for non-production build:', + 'Skipping stats collection for development build:', src ); } From 88d24cd84e844f64ac0ee7e554c9a1560309dd3a Mon Sep 17 00:00:00 2001 From: James Nylen Date: Thu, 3 Aug 2017 11:56:16 -0400 Subject: [PATCH 06/11] Add unit tests --- editor/utils/test/tracking.js | 83 +++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 editor/utils/test/tracking.js diff --git a/editor/utils/test/tracking.js b/editor/utils/test/tracking.js new file mode 100644 index 00000000000000..8c0b11f9f3a0cb --- /dev/null +++ b/editor/utils/test/tracking.js @@ -0,0 +1,83 @@ +/* eslint-disable no-console */ + +/** + * Internal dependencies + */ +import { bumpStat } from '../tracking'; + +describe( 'bumpStat', () => { + const originalConsoleError = console.error; + const originalGetUserSetting = window.getUserSetting; + + beforeEach( () => { + console.error = jest.fn(); + window.getUserSetting = () => 'off'; + } ); + + afterEach( () => { + console.error = originalConsoleError; + window.getUserSetting = originalGetUserSetting; + } ); + + it( 'should reject non-string stat groups', () => { + expect( bumpStat( 42, 'valid-name' ) ).toBeUndefined(); + expect( console.error ).toHaveBeenCalledWith( + 'Stat group names and stat names must be strings.' + ); + } ); + + it( 'should reject non-string stat names', () => { + expect( bumpStat( 'valid_group', 42 ) ).toBeUndefined(); + expect( console.error ).toHaveBeenCalledWith( + 'Stat group names and stat names must be strings.' + ); + } ); + + it( 'should reject group names with invalid characters', () => { + expect( bumpStat( 'invalid-group', 'valid-name' ) ).toBeUndefined(); + expect( console.error ).toHaveBeenCalledWith( + 'Stat group names must consist of letters, numbers, and underscores.' + ); + } ); + + it( 'should reject group names longer than 22 chars', () => { + expect( bumpStat( Array( 23 + 1 ).join( 'x' ), 'valid-name' ) ).toBeUndefined(); + expect( console.error ).toHaveBeenCalledWith( + 'Stat group names cannot be longer than 22 characters.' + ); + } ); + + it( 'should reject stat names with invalid characters', () => { + expect( bumpStat( 'group', 'invalidName' ) ).toBeUndefined(); + expect( console.error ).toHaveBeenCalledWith( + 'Stat names must consist of letters, numbers, underscores, and dashes.' + ); + } ); + + it( 'should reject stat names longer than 32 chars', () => { + expect( bumpStat( 'name', Array( 33 + 1 ).join( 'x' ) ) ).toBeUndefined(); + expect( console.error ).toHaveBeenCalledWith( + 'Stat names cannot be longer than 32 characters.' + ); + } ); + + it( 'should do nothing if the user has not opted in', () => { + expect( bumpStat( 'valid_group', 'valid-name' ) ).toBeUndefined(); + expect( console.error ).not.toHaveBeenCalled(); + } ); + + it( 'should bump valid stats', () => { + window.getUserSetting = () => 'on'; + const url = bumpStat( 'valid_group', 'valid-name' ); + // There are a couple of pieces of the URL where we don't care about + // testing the specific value. Replace them with placeholders. + const urlMatch = url + .replace( /^[a-z]+:/, 'PROTOCOL:' ) + .replace( /t=[0-9.]+$/, 't=NUMBER' ); + expect( urlMatch ).toBe( + 'PROTOCOL://pixel.wp.com/g.gif?v=wpcom-no-pv' + + '&x_gutenberg_valid_group=valid-name' + + '&t=NUMBER' + ); + } ); +} ); From d12951a751bec8ccd75495df52a18b43aef5a7e1 Mon Sep 17 00:00:00 2001 From: James Nylen Date: Thu, 3 Aug 2017 13:44:22 -0400 Subject: [PATCH 07/11] Clarify tracking prompt to indicate that post content is not collected --- editor/enable-tracking-prompt/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor/enable-tracking-prompt/index.js b/editor/enable-tracking-prompt/index.js index f7f821c068a4f8..ea24bef1d55090 100644 --- a/editor/enable-tracking-prompt/index.js +++ b/editor/enable-tracking-prompt/index.js @@ -43,7 +43,7 @@ function EnableTrackingPrompt( props ) {
- { __( 'Usage data is completely anonymous and will only be used to improve the editor.' ) } + { __( 'Usage data is completely anonymous, does not include your post content, and will only be used to improve the editor.' ) }
); From 07755f63ec221bafc8e4e2c997009c477bd9c96e Mon Sep 17 00:00:00 2001 From: James Nylen Date: Thu, 3 Aug 2017 13:56:18 -0400 Subject: [PATCH 08/11] Add tests for new `Button` features --- components/button/test/index.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/components/button/test/index.js b/components/button/test/index.js index 05224da7ce1d40..058e0f1743ca3a 100644 --- a/components/button/test/index.js +++ b/components/button/test/index.js @@ -17,7 +17,6 @@ describe( 'Button', () => { expect( button.hasClass( 'button-large' ) ).toBe( false ); expect( button.hasClass( 'button-primary' ) ).toBe( false ); expect( button.hasClass( 'is-toggled' ) ).toBe( false ); - expect( button.hasClass( 'is-borderless' ) ).toBe( false ); expect( button.prop( 'disabled' ) ).toBeUndefined(); expect( button.prop( 'type' ) ).toBe( 'button' ); expect( button.type() ).toBe( 'button' ); @@ -30,6 +29,13 @@ describe( 'Button', () => { expect( button.hasClass( 'button-primary' ) ).toBe( true ); } ); + it( 'should render a button element with button-secondary class', () => { + const button = shallow( diff --git a/editor/enable-tracking-prompt/test/index.js b/editor/enable-tracking-prompt/test/index.js index f97c66e1ae7d5c..686fa0ec0dade2 100644 --- a/editor/enable-tracking-prompt/test/index.js +++ b/editor/enable-tracking-prompt/test/index.js @@ -56,7 +56,7 @@ describe( 'EnableTrackingPrompt', () => { expect( window.setUserSetting ) .toHaveBeenCalledWith( 'gutenberg_tracking', 'on' ); expect( tracking.bumpStat ) - .not.toHaveBeenCalled(); + .toHaveBeenCalledWith( 'tracking', 'opt-in' ); expect( removeNotice ) .toHaveBeenCalledWith( TRACKING_PROMPT_NOTICE_ID ); } );