diff --git a/package-lock.json b/package-lock.json index 95a90f00ae44c3..61ee2b46523e2a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -51881,6 +51881,7 @@ "@wordpress/deprecated": "file:../deprecated", "@wordpress/dom": "file:../dom", "@wordpress/element": "file:../element", + "@wordpress/escape-html": "file:../escape-html", "@wordpress/fields": "file:../fields", "@wordpress/global-styles-engine": "file:../global-styles-engine", "@wordpress/global-styles-ui": "file:../global-styles-ui", diff --git a/packages/editor/package.json b/packages/editor/package.json index f687023eea55fa..e0081aa4ad7e4b 100644 --- a/packages/editor/package.json +++ b/packages/editor/package.json @@ -76,6 +76,7 @@ "@wordpress/deprecated": "file:../deprecated", "@wordpress/dom": "file:../dom", "@wordpress/element": "file:../element", + "@wordpress/escape-html": "file:../escape-html", "@wordpress/fields": "file:../fields", "@wordpress/global-styles-engine": "file:../global-styles-engine", "@wordpress/global-styles-ui": "file:../global-styles-ui", diff --git a/packages/editor/src/components/save-error-details/style.scss b/packages/editor/src/components/save-error-details/style.scss new file mode 100644 index 00000000000000..ae3da4bc79fbd7 --- /dev/null +++ b/packages/editor/src/components/save-error-details/style.scss @@ -0,0 +1,13 @@ +.editor-save-error-details { + display: inline; +} + +.editor-save-error-details summary { + display: inline; + cursor: var(--wpds-cursor-control); + margin-left: 0.35em; +} + +.editor-save-error-details__message { + white-space: break-spaces; +} diff --git a/packages/editor/src/store/actions.js b/packages/editor/src/store/actions.js index 30ba3529f43089..3ab8eb61aa0af7 100644 --- a/packages/editor/src/store/actions.js +++ b/packages/editor/src/store/actions.js @@ -3,6 +3,7 @@ */ import { speak } from '@wordpress/a11y'; import apiFetch from '@wordpress/api-fetch'; +import { escapeHTML } from '@wordpress/escape-html'; import deprecated from '@wordpress/deprecated'; import { parse, @@ -38,6 +39,11 @@ import { getCanvasWidthByDeviceType } from '../utils/device-type'; const { getEntitySnapshot } = unlock( coreDataPrivateApis ); +// Error codes whose message is generated by the client rather than returned by +// the server. Their messages restate what the notice already says, so there are +// no details worth disclosing. +const CLIENT_GENERATED_ERROR_CODES = [ 'offline_error', 'fetch_error' ]; + /** * Returns an action generator used in signalling that editor has initialized with * the specified post object and editor settings. @@ -294,8 +300,33 @@ export const savePost = post: previousRecord, edits, error, + options, } ); if ( args.length ) { + const [ noticeMessage ] = args; + if ( + error.message && + ! CLIENT_GENERATED_ERROR_CODES.includes( error.code ) && + ! /<\/?[^>]*>/.test( error.message ) + ) { + args[ 0 ] = `${ escapeHTML( + noticeMessage + ) }
${ escapeHTML( + __( 'Show details' ) + ) }${ escapeHTML( + error.message + ) }
`; + args[ 1 ] = { + ...args[ 1 ], + __unstableHTML: true, + speak: false, + }; + // The notices store doesn't support a separate spoken message + // when rendering raw HTML content, so announce the plain text + // manually to avoid reading the markup aloud. + speak( noticeMessage, 'assertive' ); + } + registry.dispatch( noticesStore ).createErrorNotice( ...args ); } } else { diff --git a/packages/editor/src/store/test/actions.js b/packages/editor/src/store/test/actions.js index e7051e09dc1c7d..1fb03398d47aa1 100644 --- a/packages/editor/src/store/test/actions.js +++ b/packages/editor/src/store/test/actions.js @@ -1,6 +1,7 @@ /** * WordPress dependencies */ +import { speak } from '@wordpress/a11y'; import apiFetch from '@wordpress/api-fetch'; import { store as blockEditorStore } from '@wordpress/block-editor'; import { store as coreStore } from '@wordpress/core-data'; @@ -8,6 +9,10 @@ import { createRegistry } from '@wordpress/data'; import { store as noticesStore } from '@wordpress/notices'; import { store as preferencesStore } from '@wordpress/preferences'; +jest.mock( '@wordpress/a11y', () => ( { + speak: jest.fn(), +} ) ); + /** * Internal dependencies */ @@ -268,6 +273,135 @@ describe( 'Post actions', () => { }, ] ); } ); + + it( 'adds a details action for save failures with a plain-text error message', async () => { + const post = { + id: postId, + type: 'post', + title: 'bar', + content: 'bar', + excerpt: 'crackers', + status: 'publish', + }; + + apiFetch.setFetchHandler( async ( options ) => { + const method = getMethod( options ); + const { path } = options; + + if ( + method === 'PUT' && + path.startsWith( `/wp/v2/posts/${ postId }` ) + ) { + throw { + code: 'test_save_failure', + message: 'Details from server.', + }; + } + + throw { + code: 'unknown_path', + message: `Unknown path: ${ method } ${ path }`, + }; + } ); + + const registry = createRegistryWithStores(); + + registry + .dispatch( coreStore ) + .receiveEntityRecords( 'postType', 'post', post ); + + registry.dispatch( editorStore ).setupEditor( post, { + content: 'new bar', + } ); + + await registry.dispatch( editorStore ).savePost(); + + const [ notice ] = registry.select( noticesStore ).getNotices(); + expect( notice ).toMatchObject( { + status: 'error', + __unstableHTML: true, + } ); + expect( notice.content ).toContain( + 'Updating failed. We’ll try to save a backup in this browser. Please try updating again.' + ); + expect( notice.content ).toContain( + '
' + ); + expect( notice.content ).toContain( + 'Show details' + ); + expect( notice.content ).toContain( 'Details from server.' ); + expect( notice.actions ).toEqual( [] ); + expect( notice.spokenMessage ).toBeNull(); + expect( notice.content ).toContain( + 'Details from server.' + ); + expect( speak ).toHaveBeenCalledWith( + 'Updating failed. We’ll try to save a backup in this browser. Please try updating again.', + 'assertive' + ); + } ); + + // These errors are generated by the client rather than returned by the + // server, so their message only restates the notice itself. + [ + [ + 'offline_error', + 'Unable to connect. Please check your Internet connection.', + 'Updating failed because you were offline. Please verify your connection and try again.', + ], + [ + 'fetch_error', + 'Could not get a valid response from the server.', + 'Updating failed. We’ll try to save a backup in this browser. Please try updating again.', + ], + ].forEach( ( [ code, message, expectedNotice ] ) => { + it( `omits the details disclosure for ${ code } failures`, async () => { + const post = { + id: postId, + type: 'post', + title: 'bar', + content: 'bar', + excerpt: 'crackers', + status: 'publish', + }; + + apiFetch.setFetchHandler( async ( options ) => { + const method = getMethod( options ); + const { path } = options; + + if ( + method === 'PUT' && + path.startsWith( `/wp/v2/posts/${ postId }` ) + ) { + throw { code, message }; + } + + throw { + code: 'unknown_path', + message: `Unknown path: ${ method } ${ path }`, + }; + } ); + + const registry = createRegistryWithStores(); + + registry + .dispatch( coreStore ) + .receiveEntityRecords( 'postType', 'post', post ); + + registry.dispatch( editorStore ).setupEditor( post, { + content: 'new bar', + } ); + + await registry.dispatch( editorStore ).savePost(); + + const [ notice ] = registry.select( noticesStore ).getNotices(); + expect( notice.content ).toBe( expectedNotice ); + expect( notice.content ).not.toContain( ' { diff --git a/packages/editor/src/store/utils/notice-builder.js b/packages/editor/src/store/utils/notice-builder.js index 92e68f4f8bdb3d..f02014aab2177e 100644 --- a/packages/editor/src/store/utils/notice-builder.js +++ b/packages/editor/src/store/utils/notice-builder.js @@ -8,6 +8,14 @@ import { __ } from '@wordpress/i18n'; */ import { ATTACHMENT_POST_TYPE } from '../constants'; +const AUTO_SAVE_FAILURE_NOTICE = __( + 'Auto-save failed. We’ll try to save a backup in this browser. You can also save manually.' +); + +const AUTO_SAVE_OFFLINE_FAILURE_NOTICE = __( + 'Auto-save failed because you were offline. We’ll try to save a backup in this browser. Please verify your connection and save manually.' +); + /** * Builds the arguments for a success notification dispatch. * @@ -89,7 +97,7 @@ export function getNotificationArgumentsForSaveSuccess( data ) { * notification should be sent. */ export function getNotificationArgumentsForSaveFail( data ) { - const { post, edits, error } = data; + const { post, edits, error, options } = data; if ( error && 'rest_autosave_no_changes' === error.code ) { // Autosave requested a new autosave, but there were no changes. This shouldn't // result in an error notice for the user. @@ -115,19 +123,31 @@ export function getNotificationArgumentsForSaveFail( data ) { ), }; - const noticeMessage = + let noticeMessage = ! isPublished && edits.status in messages ? messages[ edits.status ] : messages.default; + if ( options?.isAutosave ) { + noticeMessage = AUTO_SAVE_OFFLINE_FAILURE_NOTICE; + } + return [ noticeMessage, { id: 'editor-save' } ]; } const messages = { - publish: __( 'Publishing failed.' ), - private: __( 'Publishing failed.' ), - future: __( 'Scheduling failed.' ), - default: __( 'Updating failed.' ), + publish: __( + 'Publishing failed. We’ll try to save a backup in this browser. Please try publishing again.' + ), + private: __( + 'Publishing failed. We’ll try to save a backup in this browser. Please try publishing again.' + ), + future: __( + 'Scheduling failed. We’ll try to save a backup in this browser. Please try scheduling again.' + ), + default: __( + 'Updating failed. We’ll try to save a backup in this browser. Please try updating again.' + ), }; let noticeMessage = @@ -135,11 +155,10 @@ export function getNotificationArgumentsForSaveFail( data ) { ? messages[ edits.status ] : messages.default; - // Check if message string contains HTML. Notice text is currently only - // supported as plaintext, and stripping the tags may muddle the meaning. - if ( error.message && ! /<\/?[^>]*>/.test( error.message ) ) { - noticeMessage = [ noticeMessage, error.message ].join( ' ' ); + if ( options?.isAutosave ) { + noticeMessage = AUTO_SAVE_FAILURE_NOTICE; } + return [ noticeMessage, { diff --git a/packages/editor/src/store/utils/test/notice-builder.js b/packages/editor/src/store/utils/test/notice-builder.js index dddbd0c18222e5..d3d3a913765983 100644 --- a/packages/editor/src/store/utils/test/notice-builder.js +++ b/packages/editor/src/store/utils/test/notice-builder.js @@ -120,7 +120,7 @@ describe( 'getNotificationArgumentsForSaveFail()', () => { '', [ 'draft', 'publish' ], [ - 'Publishing failed. Something went wrong.', + 'Publishing failed. We’ll try to save a backup in this browser. Please try publishing again.', defaultExpectedAction, ], ], @@ -129,7 +129,7 @@ describe( 'getNotificationArgumentsForSaveFail()', () => { '', [ 'draft', 'private' ], [ - 'Publishing failed. Something went wrong.', + 'Publishing failed. We’ll try to save a backup in this browser. Please try publishing again.', defaultExpectedAction, ], ], @@ -138,7 +138,7 @@ describe( 'getNotificationArgumentsForSaveFail()', () => { '', [ 'draft', 'future' ], [ - 'Scheduling failed. Something went wrong.', + 'Scheduling failed. We’ll try to save a backup in this browser. Please try scheduling again.', defaultExpectedAction, ], ], @@ -146,7 +146,20 @@ describe( 'getNotificationArgumentsForSaveFail()', () => { 'when post is published and edits is published', '', [ 'publish', 'publish' ], - [ 'Updating failed. Something went wrong.', defaultExpectedAction ], + [ + 'Updating failed. We’ll try to save a backup in this browser. Please try updating again.', + defaultExpectedAction, + ], + ], + [ + 'when the save is an autosave', + '', + [ 'publish', 'publish' ], + [ + 'Auto-save failed. We’ll try to save a backup in this browser. You can also save manually.', + defaultExpectedAction, + ], + { isAutosave: true }, ], ].forEach( ( [ @@ -154,6 +167,7 @@ describe( 'getNotificationArgumentsForSaveFail()', () => { errorCode, [ postStatus, editsStatus ], expectedValue, + options = {}, ] ) => { // eslint-disable-next-line jest/valid-title it( description, () => { @@ -165,6 +179,7 @@ describe( 'getNotificationArgumentsForSaveFail()', () => { post, edits, error, + options, } ) ).toEqual( expectedValue ); } ); diff --git a/packages/editor/src/style.scss b/packages/editor/src/style.scss index a4a451ac33e3a7..72d1380c80468f 100644 --- a/packages/editor/src/style.scss +++ b/packages/editor/src/style.scss @@ -58,6 +58,7 @@ @use "./components/post-trash/style.scss" as *; @use "./components/preview-dropdown/style.scss" as *; @use "./components/resizable-editor/style.scss" as *; +@use "./components/save-error-details/style.scss" as *; @use "./components/save-publish-panels/style.scss" as *; @use "./components/start-page-options/style.scss" as *; @use "./components/start-template-options/style.scss" as *; diff --git a/packages/editor/tsconfig.json b/packages/editor/tsconfig.json index 3010fd8c62919c..b495a3ebe97e47 100644 --- a/packages/editor/tsconfig.json +++ b/packages/editor/tsconfig.json @@ -21,6 +21,7 @@ { "path": "../deprecated" }, { "path": "../dom" }, { "path": "../element" }, + { "path": "../escape-html" }, { "path": "../fields" }, { "path": "../global-styles-engine" }, { "path": "../global-styles-ui" },