From c50e3915a65c5f74af65ae3011bad23bdc6e322d Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Mon, 24 Apr 2017 18:32:45 +0300 Subject: [PATCH 1/3] State: Normalization and sanitization for infinite scroll settings --- client/state/jetpack/settings/test/utils.js | 103 ++++++++++++++++++-- client/state/jetpack/settings/utils.js | 26 ++++- 2 files changed, 115 insertions(+), 14 deletions(-) diff --git a/client/state/jetpack/settings/test/utils.js b/client/state/jetpack/settings/test/utils.js index e1c6f200e64e0..5de5c634dfc88 100644 --- a/client/state/jetpack/settings/test/utils.js +++ b/client/state/jetpack/settings/test/utils.js @@ -76,6 +76,53 @@ describe( 'utils', () => { jetpack_protect_global_whitelist: '123.123.123.123\n213.123.213.123', } ); } ); + + it( 'should skip infinite scroll settings when module activation state is missing', () => { + const settings = { + some_setting: 'example', + infinite_scroll: true, + }; + + expect( normalizeSettings( settings ) ).to.eql( { + some_setting: 'example', + } ); + } ); + + it( 'should set infinite_scroll to default and infinite-scroll to false if the module is inactive', () => { + const settings = { + infinite_scroll: true, + 'infinite-scroll': false, + }; + + expect( normalizeSettings( settings ) ).to.eql( { + infinite_scroll: 'default', + 'infinite-scroll': false, + } ); + } ); + + it( 'should set infinite_scroll to scroll and infinite-scroll to true if the module is active and scroll is enabled', () => { + const settings = { + infinite_scroll: true, + 'infinite-scroll': true, + }; + + expect( normalizeSettings( settings ) ).to.eql( { + infinite_scroll: 'scroll', + 'infinite-scroll': true, + } ); + } ); + + it( 'should set infinite_scroll to button and infinite-scroll to true if the module is active and scroll is disabled', () => { + const settings = { + infinite_scroll: false, + 'infinite-scroll': true, + }; + + expect( normalizeSettings( settings ) ).to.eql( { + infinite_scroll: 'button', + 'infinite-scroll': true, + } ); + } ); } ); describe( 'sanitizeSettings()', () => { @@ -99,15 +146,58 @@ describe( 'utils', () => { some_other_setting: 123, } ); } ); + + it( 'should skip infinite scroll settings if infinite_scroll is not defined', () => { + const settings = { + some_other_setting: 123, + 'infinite-scroll': true, + }; + + expect( sanitizeSettings( settings ) ).to.eql( { + some_other_setting: 123, + } ); + } ); + + it( 'should disable infinite scroll module when set to the default setting', () => { + const settings = { + infinite_scroll: 'default', + 'infinite-scroll': true, + }; + + expect( sanitizeSettings( settings ) ).to.eql( { + 'infinite-scroll': false, + } ); + } ); + + it( 'should enable infinite scroll module and set scroll to true when setting is scroll', () => { + const settings = { + infinite_scroll: 'scroll', + 'infinite-scroll': false, + }; + + expect( sanitizeSettings( settings ) ).to.eql( { + infinite_scroll: true, + 'infinite-scroll': true, + } ); + } ); + + it( 'should enable infinite scroll module and set scroll to false when setting is button', () => { + const settings = { + infinite_scroll: 'button', + 'infinite-scroll': false, + }; + + expect( sanitizeSettings( settings ) ).to.eql( { + infinite_scroll: false, + 'infinite-scroll': true, + } ); + } ); } ); describe( 'filterSettingsByActiveModules()', () => { it( 'should remove module activation state and retain all module settings for enabled modules', () => { const settings = { example_setting: true, - 'infinite-scroll': true, - infinite_scroll: false, - infinite_scroll_google_analytics: true, minileven: true, wp_mobile_excerpt: true, wp_mobile_featured_images: true, @@ -159,8 +249,6 @@ describe( 'utils', () => { expect( filterSettingsByActiveModules( settings ) ).to.eql( { example_setting: true, - infinite_scroll: false, - infinite_scroll_google_analytics: true, wp_mobile_excerpt: true, wp_mobile_featured_images: true, wp_mobile_app_promos: false, @@ -203,9 +291,6 @@ describe( 'utils', () => { it( 'should omit all module settings for disabled modules', () => { const settings = { example_setting: true, - 'infinite-scroll': false, - infinite_scroll: false, - infinite_scroll_google_analytics: true, minileven: false, wp_mobile_excerpt: true, wp_mobile_featured_images: true, @@ -263,8 +348,6 @@ describe( 'utils', () => { it( 'should omit all module settings for modules with unknown activation state', () => { const settings = { example_setting: true, - infinite_scroll: false, - infinite_scroll_google_analytics: true, wp_mobile_excerpt: true, wp_mobile_featured_images: true, wp_mobile_app_promos: false, diff --git a/client/state/jetpack/settings/utils.js b/client/state/jetpack/settings/utils.js index ceeb7d93bd75f..fbc5a841d3d8e 100644 --- a/client/state/jetpack/settings/utils.js +++ b/client/state/jetpack/settings/utils.js @@ -19,6 +19,18 @@ export const normalizeSettings = ( settings ) => { const whitelist = get( settings[ key ], [ 'local' ], [] ); memo[ key ] = whitelist.join( '\n' ); break; + case 'infinite-scroll': + break; + case 'infinite_scroll': + if ( settings[ 'infinite-scroll' ] !== undefined ) { + if ( settings[ 'infinite-scroll' ] ) { + memo[ key ] = settings[ key ] ? 'scroll' : 'button'; + } else { + memo[ key ] = 'default'; + } + memo[ 'infinite-scroll' ] = settings[ 'infinite-scroll' ]; + } + break; default: memo[ key ] = settings[ key ]; } @@ -38,6 +50,16 @@ export const sanitizeSettings = ( settings ) => { switch ( key ) { case 'post_by_email_address': break; + case 'infinite-scroll': + break; + case 'infinite_scroll': + if ( settings[ key ] === 'default' ) { + memo[ 'infinite-scroll' ] = false; + } else { + memo[ 'infinite-scroll' ] = true; + memo[ key ] = settings[ key ] === 'scroll'; + } + break; default: memo[ key ] = settings[ key ]; } @@ -54,10 +76,6 @@ export const sanitizeSettings = ( settings ) => { */ export const filterSettingsByActiveModules = ( settings ) => { const moduleSettingsList = { - 'infinite-scroll': [ - 'infinite_scroll', - 'infinite_scroll_google_analytics', - ], minileven: [ 'wp_mobile_excerpt', 'wp_mobile_featured_images', From 162d7fbe27729b1ac8057e1bc2f54ea75ebe14c7 Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Mon, 24 Apr 2017 18:33:25 +0300 Subject: [PATCH 2/3] Site Settings: Introduce handleAutosavingRadio in wrapSettingsForm --- .../my-sites/site-settings/wrap-settings-form.jsx | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/client/my-sites/site-settings/wrap-settings-form.jsx b/client/my-sites/site-settings/wrap-settings-form.jsx index 0d1f85ba9c326..506512d0250e2 100644 --- a/client/my-sites/site-settings/wrap-settings-form.jsx +++ b/client/my-sites/site-settings/wrap-settings-form.jsx @@ -122,9 +122,22 @@ const wrapSettingsForm = getFormSettings => SettingsForm => { const currentTargetName = event.currentTarget.name, currentTargetValue = event.currentTarget.value; + this.props.trackEvent( `Set ${ currentTargetName } to ${ currentTargetValue }` ); this.props.updateFields( { [ currentTargetName ]: currentTargetValue } ); }; + handleAutosavingRadio = ( name, value ) => () => { + const { fields } = this.props; + if ( fields[ name ] === value ) { + return; + } + + this.props.trackEvent( `Set ${ name } to ${ value }` ); + this.props.updateFields( { [ name ]: value }, () => { + this.submitForm(); + } ); + }; + handleSelect = event => { const { name, value } = event.currentTarget; // Attempt to cast numeric fields value to int @@ -177,6 +190,7 @@ const wrapSettingsForm = getFormSettings => SettingsForm => { handleSubmitForm: this.handleSubmitForm, handleToggle: this.handleToggle, handleAutosavingToggle: this.handleAutosavingToggle, + handleAutosavingRadio: this.handleAutosavingRadio, onChangeField: this.onChangeField, setFieldValue: this.setFieldValue, submitForm: this.submitForm, From a8e8ccd95ac79a6200538244175f2307086b8333 Mon Sep 17 00:00:00 2001 From: Marin Atanasov Date: Mon, 24 Apr 2017 18:34:48 +0300 Subject: [PATCH 3/3] Site Settings: Refactor Infinite Scroll settings to Radio --- .../my-sites/site-settings/form-writing.jsx | 3 +- client/my-sites/site-settings/style.scss | 4 ++ .../site-settings/theme-enhancements.jsx | 65 ++++++++++++------- 3 files changed, 46 insertions(+), 26 deletions(-) diff --git a/client/my-sites/site-settings/form-writing.jsx b/client/my-sites/site-settings/form-writing.jsx index e25860cc144ec..e96e5a66ad4c1 100644 --- a/client/my-sites/site-settings/form-writing.jsx +++ b/client/my-sites/site-settings/form-writing.jsx @@ -55,6 +55,7 @@ class SiteSettingsFormWriting extends Component { handleSelect, handleToggle, handleAutosavingToggle, + handleAutosavingRadio, isRequestingSettings, isSavingSettings, onChangeField, @@ -137,6 +138,7 @@ class SiteSettingsFormWriting extends Component { { 'jetpack_portfolio', 'infinite-scroll', 'infinite_scroll', - 'infinite_scroll_google_analytics', 'minileven', 'wp_mobile_excerpt', 'wp_mobile_featured_images', diff --git a/client/my-sites/site-settings/style.scss b/client/my-sites/site-settings/style.scss index 28516d461ef29..20006ca1d0067 100644 --- a/client/my-sites/site-settings/style.scss +++ b/client/my-sites/site-settings/style.scss @@ -377,6 +377,10 @@ float: right; } +.form-legend + .site-settings__info-link-container { + margin-top: -21px; +} + .site-settings__add-to-whitelist { margin-top: 3px; } diff --git a/client/my-sites/site-settings/theme-enhancements.jsx b/client/my-sites/site-settings/theme-enhancements.jsx index d449097f581e4..ed25ec51c4ab1 100644 --- a/client/my-sites/site-settings/theme-enhancements.jsx +++ b/client/my-sites/site-settings/theme-enhancements.jsx @@ -12,6 +12,9 @@ import SectionHeader from 'components/section-header'; import Card from 'components/card'; import JetpackModuleToggle from 'my-sites/site-settings/jetpack-module-toggle'; import FormFieldset from 'components/forms/form-fieldset'; +import FormLegend from 'components/forms/form-legend'; +import FormLabel from 'components/forms/form-label'; +import FormRadio from 'components/forms/form-radio'; import CompactFormToggle from 'components/forms/form-toggle/compact'; import { getSelectedSiteId } from 'state/ui/selectors'; import { isJetpackModuleActive } from 'state/selectors'; @@ -28,6 +31,7 @@ class ThemeEnhancements extends Component { static propTypes = { onSubmitForm: PropTypes.func.isRequired, handleAutosavingToggle: PropTypes.func.isRequired, + handleAutosavingRadio: PropTypes.func.isRequired, isSavingSettings: PropTypes.bool, isRequestingSettings: PropTypes.bool, fields: PropTypes.object, @@ -55,16 +59,31 @@ class ThemeEnhancements extends Component { ); } + renderRadio( name, value, label ) { + const { fields, handleAutosavingRadio } = this.props; + return ( + + + { label } + + ); + } + renderInfiniteScrollSettings() { - const { - selectedSiteId, - infiniteScrollModuleActive, - translate - } = this.props; - const formPending = this.isFormPending(); + const { translate } = this.props; return ( + + { translate( 'Infinite Scroll' ) } + +
@@ -73,25 +92,21 @@ class ThemeEnhancements extends Component {
- - -
- { - this.renderToggle( 'infinite_scroll', ! infiniteScrollModuleActive, translate( - 'Scroll infinitely (Shows 7 posts on each load)' - ) ) - } - { - this.renderToggle( 'infinite_scroll_google_analytics', ! infiniteScrollModuleActive, translate( - 'Track each infinite Scroll post load as a page view in Google Analytics' - ) ) - } -
+ { + this.renderRadio( 'infinite_scroll', 'default', translate( + 'Load more posts using the default theme behavior' + ) ) + } + { + this.renderRadio( 'infinite_scroll', 'button', translate( + 'Load more posts in page with a button' + ) ) + } + { + this.renderRadio( 'infinite_scroll', 'scroll', translate( + 'Load more posts as the reader scrolls down' + ) ) + } ); }