From ee7306b1b7745aca4bef544f80383d7349fb5a9c Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Fri, 5 Jul 2019 14:06:06 +1000 Subject: [PATCH 01/12] Add confirmation step to Custom Fields option Add a confirmation step to the Custom Fields option so that the page doesn't reload without warning. Co-authored-by: Marek Hrabe --- .../components/options-modal/options/base.js | 16 +- .../options/enable-custom-fields.js | 70 ++++---- .../enable-custom-fields.js.snap | 158 ++++++++++++++++-- .../options/test/enable-custom-fields.js | 73 ++++---- .../src/components/options-modal/style.scss | 21 ++- 5 files changed, 247 insertions(+), 91 deletions(-) diff --git a/packages/edit-post/src/components/options-modal/options/base.js b/packages/edit-post/src/components/options-modal/options/base.js index 1dd5b9d013af0..cef8fe130b268 100644 --- a/packages/edit-post/src/components/options-modal/options/base.js +++ b/packages/edit-post/src/components/options-modal/options/base.js @@ -3,14 +3,16 @@ */ import { CheckboxControl } from '@wordpress/components'; -function BaseOption( { label, isChecked, onChange } ) { +function BaseOption( { label, isChecked, onChange, children } ) { return ( - +
+ + { children } +
); } diff --git a/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js b/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js index 140c6f1c46d2d..a3ef5e3e35677 100644 --- a/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js +++ b/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js @@ -1,7 +1,9 @@ /** * WordPress dependencies */ -import { Component } from '@wordpress/element'; +import { useState } from '@wordpress/element'; +import { __ } from '@wordpress/i18n'; +import { Notice, Button } from '@wordpress/components'; import { withSelect } from '@wordpress/data'; /** @@ -9,39 +11,43 @@ import { withSelect } from '@wordpress/data'; */ import BaseOption from './base'; -export class EnableCustomFieldsOption extends Component { - constructor( { isChecked } ) { - super( ...arguments ); - - this.toggleCustomFields = this.toggleCustomFields.bind( this ); - - this.state = { isChecked }; - } - - toggleCustomFields() { - // Submit a hidden form which triggers the toggle_custom_fields admin action. - // This action will toggle the setting and reload the editor with the meta box - // assets included on the page. - document.getElementById( 'toggle-custom-fields-form' ).submit(); - - // Make it look like something happened while the page reloads. - this.setState( { isChecked: ! this.props.isChecked } ); - } - - render() { - const { label } = this.props; - const { isChecked } = this.state; +export function CustomFieldsConfirmation() { + const [ isReloading, setIsReloading ] = useState( false ); + + return ( +
+ + { __( 'Page reload is required for this change.' ) } + + +
+ ); +} - return ( - - ); - } +export function EnableCustomFieldsOption( { label, areCustomFieldsEnabled } ) { + const [ isChecked, setIsChecked ] = useState( areCustomFieldsEnabled ); + + return ( + setIsChecked( nextIsChecked ) } + > + { isChecked !== areCustomFieldsEnabled && } + + ); } export default withSelect( ( select ) => ( { - isChecked: !! select( 'core/editor' ).getEditorSettings().enableCustomFields, + areCustomFieldsEnabled: !! select( 'core/editor' ).getEditorSettings().enableCustomFields, } ) )( EnableCustomFieldsOption ); diff --git a/packages/edit-post/src/components/options-modal/options/test/__snapshots__/enable-custom-fields.js.snap b/packages/edit-post/src/components/options-modal/options/test/__snapshots__/enable-custom-fields.js.snap index 14bed9e08237e..c0e3e1a926e64 100644 --- a/packages/edit-post/src/components/options-modal/options/test/__snapshots__/enable-custom-fields.js.snap +++ b/packages/edit-post/src/components/options-modal/options/test/__snapshots__/enable-custom-fields.js.snap @@ -1,17 +1,151 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`EnableCustomFieldsOption renders properly when checked 1`] = ` - +exports[`EnableCustomFieldsOption renders a checked checkbox and a confirmation message when toggled on 1`] = ` +
+
+
+ +
+
+
+
+
+ Page reload is required for this change. +
+
+ +
+
`; -exports[`EnableCustomFieldsOption renders properly when unchecked 1`] = ` - +exports[`EnableCustomFieldsOption renders a checked checkbox when custom fields are enabled 1`] = ` +
+
+
+ +
+
+
+`; + +exports[`EnableCustomFieldsOption renders an unchecked checkbox and a confirmation message when toggled off 1`] = ` +
+
+
+ +
+
+
+
+
+ Page reload is required for this change. +
+
+ +
+
+`; + +exports[`EnableCustomFieldsOption renders an unchecked checkbox when custom fields are disabled 1`] = ` +
+
+
+ +
+
+
`; diff --git a/packages/edit-post/src/components/options-modal/options/test/enable-custom-fields.js b/packages/edit-post/src/components/options-modal/options/test/enable-custom-fields.js index 8ce8f78eb4be6..5ac60c92753af 100644 --- a/packages/edit-post/src/components/options-modal/options/test/enable-custom-fields.js +++ b/packages/edit-post/src/components/options-modal/options/test/enable-custom-fields.js @@ -1,62 +1,63 @@ /** * External dependencies */ -import { shallow } from 'enzyme'; +import { default as TestRenderer, act } from 'react-test-renderer'; + +/** + * WordPress dependencies + */ +import { Button } from '@wordpress/components'; /** * Internal dependencies */ -import { EnableCustomFieldsOption } from '../enable-custom-fields'; +import { EnableCustomFieldsOption, CustomFieldsConfirmation } from '../enable-custom-fields'; +import BaseOption from '../base'; describe( 'EnableCustomFieldsOption', () => { - it( 'renders properly when checked', () => { - const wrapper = shallow( ); - expect( wrapper ).toMatchSnapshot(); + it( 'renders a checked checkbox when custom fields are enabled', () => { + const renderer = TestRenderer.create( ); + expect( renderer ).toMatchSnapshot(); } ); - it( 'can be unchecked', () => { - const submit = jest.fn(); - const getElementById = jest.spyOn( document, 'getElementById' ).mockImplementation( () => ( { - submit, - } ) ); - - const wrapper = shallow( ); - - expect( wrapper.prop( 'isChecked' ) ).toBe( true ); - - wrapper.prop( 'onChange' )(); - wrapper.update(); - - expect( wrapper.prop( 'isChecked' ) ).toBe( false ); - expect( getElementById ).toHaveBeenCalledWith( 'toggle-custom-fields-form' ); - expect( submit ).toHaveBeenCalled(); + it( 'renders an unchecked checkbox when custom fields are disabled', () => { + const renderer = TestRenderer.create( + + ); + expect( renderer ).toMatchSnapshot(); + } ); - getElementById.mockRestore(); + it( 'renders an unchecked checkbox and a confirmation message when toggled off', () => { + const renderer = new TestRenderer.create( ); + act( () => { + renderer.root.findByType( BaseOption ).props.onChange( false ); + } ); + expect( renderer ).toMatchSnapshot(); } ); - it( 'renders properly when unchecked', () => { - const wrapper = shallow( - + it( 'renders a checked checkbox and a confirmation message when toggled on', () => { + const renderer = new TestRenderer.create( + ); - expect( wrapper ).toMatchSnapshot(); + act( () => { + renderer.root.findByType( BaseOption ).props.onChange( true ); + } ); + expect( renderer ).toMatchSnapshot(); } ); +} ); - it( 'can be checked', () => { +describe( 'CustomFieldsConfirmation', () => { + it( 'submits the toggle-custom-fields-form', () => { const submit = jest.fn(); const getElementById = jest.spyOn( document, 'getElementById' ).mockImplementation( () => ( { submit, } ) ); - const wrapper = shallow( - - ); - - expect( wrapper.prop( 'isChecked' ) ).toBe( false ); - - wrapper.prop( 'onChange' )(); - wrapper.update(); + const renderer = new TestRenderer.create( ); + act( () => { + renderer.root.findByType( Button ).props.onClick(); + } ); - expect( wrapper.prop( 'isChecked' ) ).toBe( true ); expect( getElementById ).toHaveBeenCalledWith( 'toggle-custom-fields-form' ); expect( submit ).toHaveBeenCalled(); diff --git a/packages/edit-post/src/components/options-modal/style.scss b/packages/edit-post/src/components/options-modal/style.scss index 01b5f816c0ca0..4c0e6aff10e7c 100644 --- a/packages/edit-post/src/components/options-modal/style.scss +++ b/packages/edit-post/src/components/options-modal/style.scss @@ -21,13 +21,26 @@ margin: 0; } - &.components-base-control + &.components-base-control { - margin-bottom: 0; - } - .components-checkbox-control__label { flex-grow: 1; padding: 0.6rem 0 0.6rem 10px; } } + + &__custom-fields-confirmation { + margin-left: 48px; + + @include break-medium() { + margin-left: 38px; + } + + .components-notice, + .components-button { + margin: 0.2rem 0; + } + + .components-notice__content { + margin: 0; + } + } } From 1d5999d74246bacf24a2fec7922aadd9003968a7 Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Fri, 5 Jul 2019 14:52:15 +1000 Subject: [PATCH 02/12] Update E2E tests for new Custom Fields setting flow --- packages/e2e-tests/specs/preview.test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/preview.test.js b/packages/e2e-tests/specs/preview.test.js index 5a1d7f4cc2754..7d902862054b8 100644 --- a/packages/e2e-tests/specs/preview.test.js +++ b/packages/e2e-tests/specs/preview.test.js @@ -70,8 +70,10 @@ async function toggleCustomFieldsOption( shouldBeChecked ) { ); if ( isChecked !== shouldBeChecked ) { - const navigationCompleted = page.waitForNavigation(); await checkboxHandle.click(); + const [ saveButton ] = await page.$x( '//button[text()="Save & Reload"]' ); + const navigationCompleted = page.waitForNavigation(); + saveButton.click(); await navigationCompleted; return; } From daebe655625e3c0544f4331fc090a90877456920 Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Mon, 8 Jul 2019 14:50:19 +0200 Subject: [PATCH 03/12] Remove notice in favor of plain text. --- .../options/enable-custom-fields.js | 6 ++--- .../enable-custom-fields.js.snap | 24 +++++++------------ .../src/components/options-modal/style.scss | 14 ++--------- 3 files changed, 13 insertions(+), 31 deletions(-) diff --git a/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js b/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js index a3ef5e3e35677..c5d9dae4996e7 100644 --- a/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js +++ b/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js @@ -3,7 +3,7 @@ */ import { useState } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; -import { Notice, Button } from '@wordpress/components'; +import { Button } from '@wordpress/components'; import { withSelect } from '@wordpress/data'; /** @@ -16,9 +16,9 @@ export function CustomFieldsConfirmation() { return (
- +

{ __( 'Page reload is required for this change.' ) } - +

-
+ ); } diff --git a/packages/edit-post/src/components/options-modal/options/test/__snapshots__/enable-custom-fields.js.snap b/packages/edit-post/src/components/options-modal/options/test/__snapshots__/enable-custom-fields.js.snap index bef79c80e21df..d01158f4521fe 100644 --- a/packages/edit-post/src/components/options-modal/options/test/__snapshots__/enable-custom-fields.js.snap +++ b/packages/edit-post/src/components/options-modal/options/test/__snapshots__/enable-custom-fields.js.snap @@ -24,23 +24,19 @@ exports[`EnableCustomFieldsOption renders a checked checkbox and a confirmation /> -
-

- Page reload is required for this change. -

- -
+ Page reload is required for this change. +

+ `; @@ -95,23 +91,19 @@ exports[`EnableCustomFieldsOption renders an unchecked checkbox and a confirmati /> -
-

- Page reload is required for this change. -

- -
+ Page reload is required for this change. +

+ `; diff --git a/packages/edit-post/src/components/options-modal/style.scss b/packages/edit-post/src/components/options-modal/style.scss index ea30a2b77a1af..c16d4be939da1 100644 --- a/packages/edit-post/src/components/options-modal/style.scss +++ b/packages/edit-post/src/components/options-modal/style.scss @@ -27,10 +27,12 @@ } } - &__custom-fields-confirmation { - &-message, - .components-button { - margin: 0 0 0.6rem; + &__custom-fields-confirmation-message, + &__custom-fields-confirmation-button { + margin: 0 0 0.6rem 48px; + + @include break-medium() { + margin-left: 38px; } } } From 6be6aea240c9501ea3bb67d4a8a6f3896843a982 Mon Sep 17 00:00:00 2001 From: Marek Hrabe Date: Thu, 25 Jul 2019 18:45:34 +0200 Subject: [PATCH 05/12] update description Co-Authored-By: Daniel Richards --- .../components/options-modal/options/enable-custom-fields.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js b/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js index de5f2f020b6f8..880358153a916 100644 --- a/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js +++ b/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js @@ -17,7 +17,7 @@ export function CustomFieldsConfirmation() { return ( <>

- { __( 'Page reload is required for this change.' ) } + { __( 'A page reload is required for this change.' ) }

); @@ -44,7 +44,7 @@ export function EnableCustomFieldsOption( { label, areCustomFieldsEnabled } ) { isChecked={ isChecked } onChange={ setIsChecked } > - { isChecked !== areCustomFieldsEnabled && } + { isChecked !== areCustomFieldsEnabled && } ); } From 4079f8095b31e20f4d252beaf33a38233ffaad37 Mon Sep 17 00:00:00 2001 From: Marek Hrabe Date: Sat, 27 Jul 2019 09:38:17 -0700 Subject: [PATCH 08/12] add notice about saved content and make sure it won't expand modal --- .../components/options-modal/options/enable-custom-fields.js | 2 +- packages/edit-post/src/components/options-modal/style.scss | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js b/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js index 4badbbe6ceab7..01e09febe20bd 100644 --- a/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js +++ b/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js @@ -17,7 +17,7 @@ export function CustomFieldsConfirmation( { nextState } ) { return ( <>

- { __( 'A page reload is required for this change.' ) } + { __( 'A page reload is required for this change. Make sure your content is saved before reloading.' ) }

`; @@ -94,7 +94,7 @@ exports[`EnableCustomFieldsOption renders an unchecked checkbox and a confirmati

- Page reload is required for this change. + A page reload is required for this change. Make sure your content is saved before reloading.

`; From bba485f3736bf42a3f2b830ca16a970962fcf04d Mon Sep 17 00:00:00 2001 From: Marek Hrabe Date: Mon, 29 Jul 2019 11:14:50 -0700 Subject: [PATCH 10/12] make button label in test dynamic --- packages/e2e-tests/specs/preview.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/preview.test.js b/packages/e2e-tests/specs/preview.test.js index 7d902862054b8..4f1fa04ba06eb 100644 --- a/packages/e2e-tests/specs/preview.test.js +++ b/packages/e2e-tests/specs/preview.test.js @@ -71,7 +71,7 @@ async function toggleCustomFieldsOption( shouldBeChecked ) { if ( isChecked !== shouldBeChecked ) { await checkboxHandle.click(); - const [ saveButton ] = await page.$x( '//button[text()="Save & Reload"]' ); + const [ saveButton ] = await page.$x( shouldBeChecked ? '//button[text()="Enable & Reload"]' : '//button[text()="Disable & Reload"]' ); const navigationCompleted = page.waitForNavigation(); saveButton.click(); await navigationCompleted; From 9bf2985e158c043c59182f596a02360ef4fd98e6 Mon Sep 17 00:00:00 2001 From: Marek Hrabe Date: Wed, 31 Jul 2019 21:01:56 -0700 Subject: [PATCH 11/12] simplify condition for determining label Co-Authored-By: Robert Anderson --- .../components/options-modal/options/enable-custom-fields.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js b/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js index 01e09febe20bd..d39b78ddad000 100644 --- a/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js +++ b/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js @@ -29,7 +29,7 @@ export function CustomFieldsConfirmation( { nextState } ) { document.getElementById( 'toggle-custom-fields-form' ).submit(); } } > - { nextState === true ? __( 'Enable & Reload' ) : __( 'Disable & Reload' ) } + { nextState ? __( 'Enable & Reload' ) : __( 'Disable & Reload' ) } ); From 929aeaf37050f1eceb176ee51affd6e277c0a028 Mon Sep 17 00:00:00 2001 From: Marek Hrabe Date: Wed, 31 Jul 2019 21:03:33 -0700 Subject: [PATCH 12/12] use willEnable as the prop name --- .../options-modal/options/enable-custom-fields.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js b/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js index d39b78ddad000..82b8002dca148 100644 --- a/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js +++ b/packages/edit-post/src/components/options-modal/options/enable-custom-fields.js @@ -11,7 +11,7 @@ import { withSelect } from '@wordpress/data'; */ import BaseOption from './base'; -export function CustomFieldsConfirmation( { nextState } ) { +export function CustomFieldsConfirmation( { willEnable } ) { const [ isReloading, setIsReloading ] = useState( false ); return ( @@ -29,7 +29,7 @@ export function CustomFieldsConfirmation( { nextState } ) { document.getElementById( 'toggle-custom-fields-form' ).submit(); } } > - { nextState ? __( 'Enable & Reload' ) : __( 'Disable & Reload' ) } + { willEnable ? __( 'Enable & Reload' ) : __( 'Disable & Reload' ) } ); @@ -44,7 +44,7 @@ export function EnableCustomFieldsOption( { label, areCustomFieldsEnabled } ) { isChecked={ isChecked } onChange={ setIsChecked } > - { isChecked !== areCustomFieldsEnabled && } + { isChecked !== areCustomFieldsEnabled && } ); }