Fix issue with cut (#3246)

assets/src/stories-editor/blocks/amp-story-page/copy-paste-handler.js

@@ -13,22 +13,25 @@ import { withDispatch, useSelect, useDispatch } from '@wordpress/data';
 /**
  * Internal dependencies
  */
-import { ensureAllowedBlocksOnPaste } from '../../helpers';
+import { copyTextToClipBoard, ensureAllowedBlocksOnPaste, isPageBlock } from '../../helpers';
 
-function CopyPasteHandler( { children, onCopy, onCut, clientId, isSelected } ) {
+function CopyPasteHandler( { children, onCopy, clientId, isSelected } ) {
 	const {
 		isFirstPage,
 		canUserUseUnfilteredHTML,
+		getCopiedMarkupState,
 	} = useSelect(
 		( select ) => {
 			const {
 				getBlockOrder,
 				getSettings,
 			} = select( 'core/block-editor' );
 			const { __experimentalCanUserUseUnfilteredHTML } = getSettings();
+			const { getCopiedMarkup } = select( 'amp/story' );
 			return {
 				isFirstPage: getBlockOrder().indexOf( clientId ) === 0,
 				canUserUseUnfilteredHTML: __experimentalCanUserUseUnfilteredHTML,
+				getCopiedMarkupState: getCopiedMarkup,
 			};
 		}, [ clientId ]
 	);
@@ -55,10 +58,9 @@ function CopyPasteHandler( { children, onCopy, onCut, clientId, isSelected } ) {
 			try {
 				html = clipboardData.getData( 'Text' );
 			} catch ( error2 ) {
-				// Some browsers like UC Browser paste plain text by default and
-				// don't support clipboardData at all, so allow default
-				// behaviour.
-				return;
+				// If everything goes wrong, fall back to state based clipboard.
+				plainText = getCopiedMarkupState();
+				html = getCopiedMarkupState();
 			}
 		}
 
@@ -79,7 +81,7 @@ function CopyPasteHandler( { children, onCopy, onCut, clientId, isSelected } ) {
 	};
 
 	return (
-		<div onCopy={ onCopy } onPaste={ onPaste } onCut={ onCut }>
+		<div onCopy={ onCopy } onPaste={ onPaste } onCut={ onCopy }>
 			{ children }
 		</div>
 	);
@@ -90,7 +92,6 @@ CopyPasteHandler.propTypes = {
 	clientId: PropTypes.string.isRequired,
 	isSelected: PropTypes.bool.isRequired,
 	onCopy: PropTypes.func.isRequired,
-	onCut: PropTypes.func.isRequired,
 };
 
 export default withDispatch( ( dispatch, ownProps, { select } ) => {
@@ -99,18 +100,19 @@ export default withDispatch( ( dispatch, ownProps, { select } ) => {
 		getSelectedBlockClientIds,
 		hasMultiSelection,
 	} = select( 'core/block-editor' );
+	const { getCurrentPage } = select( 'amp/story' );
 	const { clearCopiedMarkup, setCopiedMarkup } = dispatch( 'amp/story' );
+	const { removeBlock, selectBlock } = dispatch( 'core/block-editor' );
 
 	/**
 	 * Creates cut/copy handler for ensuring that the store's copiedMarkup is in sync with what's actually in clipBoard.
 	 * If it's not a block that's being copied, let's clear the copiedMarkup.
 	 * Otherwise, let's set the copied markup.
 	 * If it's a cut handler, finally remove the currently selected block.
 	 *
-	 * @param {boolean} isCut  Set to true if this is a cut handler, false if copy handler
-	 * @return {Function} Returns an event handler for the desired action
+	 * @param  {Event} event Event object.
 	 */
-	const createCutCopyHandler = ( isCut ) => () => {
+	const onCopy = ( event ) => {
 		const selectedBlockClientIds = getSelectedBlockClientIds();
 
 		if ( selectedBlockClientIds.length === 0 ) {
@@ -123,24 +125,36 @@ export default withDispatch( ( dispatch, ownProps, { select } ) => {
 			clearCopiedMarkup();
 			return;
 		}
-		const serialized = serialize( getBlocksByClientId( selectedBlockClientIds ) );
-		setCopiedMarkup( serialized );
-
-		if ( isCut ) {
-			// TODO: Remove selected Blocks.
-			// The code below works most of the time, but sometimes (unable to tell when and why) another element on page is selected when the block is removed, and then the browser copies this other element in stead of the previously selected element (that has now been removed).
-			/* for ( const clientId of selectedBlockClientIds ) {
-				removeBlock( clientId );
-			} */
-			// wrapping the call in setTimeout fixes the case where another element is selected on cut, but throws an error in the cases, where the above code works fine.
+
+		// Don't allow story blocks to be copyied.
+		for ( const selectedBlockClientId of selectedBlockClientIds ) {
+			if ( isPageBlock( selectedBlockClientId ) ) {
+				clearCopiedMarkup();
+				return;
+			}
 		}
-	};
 
-	const onCopy = createCutCopyHandler( false );
-	const onCut = createCutCopyHandler( true );
+		const copyBlocks = getBlocksByClientId( selectedBlockClientIds );
+		const serialized = serialize( copyBlocks );
+		// Workout what type of event, from event object passed to this function.
+		const isCut = ( event.type === 'cut' );
+
+		// Make sure that setCopiedMarkup finishes before doing anything else.
+		setCopiedMarkup( serialized ).then( () => {
+			copyTextToClipBoard( serialized );
+
+			if ( isCut ) {
+				const pageClientId = getCurrentPage();
+				for ( const clientId of selectedBlockClientIds ) {
+					// On removing block, change focus to the page, to make sure that editor doesn't get confused and tries to select an already removed block.
+					selectBlock( pageClientId );
+					removeBlock( clientId );
+				}
+			}
+		} );
+	};
 
 	return {
 		onCopy,
-		onCut,
 	};
 } )( CopyPasteHandler );

assets/src/stories-editor/components/animation-controls/index.js

@@ -97,7 +97,7 @@ const AnimationControls = ( {
 };
 
 AnimationControls.propTypes = {
-	isImageBlock: PropTypes.bool.isRequired,
+	isImageBlock: PropTypes.bool,
 	animatedBlocks: PropTypes.func.isRequired,
 	onAnimationTypeChange: PropTypes.func.isRequired,
 	onAnimationDurationChange: PropTypes.func.isRequired,

assets/src/stories-editor/components/right-click-menu/index.js

@@ -28,6 +28,7 @@ import useOutsideClickChecker from './outside-click-checker';
 import {
 	copyTextToClipBoard,
 	ensureAllowedBlocksOnPaste,
+	isPageBlock,
 } from '../../helpers';
 import { ALLOWED_MOVABLE_BLOCKS, DISABLE_DUPLICATE_BLOCKS } from '../../constants';
 
@@ -60,7 +61,6 @@ const RightClickMenu = ( props ) => {
 	const blockClientIds = castArray( clientIds );
 	const firstBlockClientId = blockClientIds[ 0 ];
 	const block = getBlock( firstBlockClientId );
-	const isPageBlock = block ? 'amp/amp-story-page' === block.name : false;
 
 	const onClose = () => {
 		setIsOpen( false );
@@ -77,7 +77,7 @@ const RightClickMenu = ( props ) => {
 	let blockActions = [];
 
 	// Don't allow any actions other than pasting with Page.
-	if ( ! isPageBlock ) {
+	if ( ! isPageBlock( firstBlockClientId ) ) {
 		blockActions = [
 			{
 				name: __( 'Copy Block', 'amp' ),
@@ -117,7 +117,7 @@ const RightClickMenu = ( props ) => {
 	}
 
 	// If it's Page block and clipboard is empty, don't display anything.
-	if ( ! getCopiedMarkup().length && isPageBlock ) {
+	if ( ! getCopiedMarkup().length && isPageBlock( firstBlockClientId ) ) {
 		return '';
 	}
 

assets/src/stories-editor/components/template-inserter/index.js

@@ -22,7 +22,7 @@ import TemplatePreview from './template-preview';
 import pageIcon from '../../../../images/stories-editor/add-page-inserter.svg';
 import addTemplateIcon from '../../../../images/stories-editor/add-template.svg';
 import './edit.css';
-import { createSkeletonTemplate, maybeEnqueueFontStyle } from '../../helpers';
+import { createSkeletonTemplate, maybeEnqueueFontStyle, isPageBlock } from '../../helpers';
 
 class TemplateInserter extends Component {
 	constructor( ...args ) {
@@ -169,13 +169,8 @@ export default compose(
 
 		const reusableBlocks = getReusableBlocks();
 
-		const isStoryBlock = ( clientId ) => {
-			const block = getBlock( clientId );
-			return block && 'amp/amp-story-page' === block.name;
-		};
-
 		return {
-			storyTemplates: reusableBlocks.filter( ( { clientId } ) => isStoryBlock( clientId ) ),
+			storyTemplates: reusableBlocks.filter( ( { clientId } ) => isPageBlock( clientId ) ),
 			getBlock,
 			allBlocks: getBlocks(),
 		};

assets/src/stories-editor/helpers/index.js

@@ -2132,3 +2132,14 @@ export const startAnimation = ( block, animationType, animationDuration, animati
 
 	blockElement.addEventListener( 'animationend', callback, { once: true } );
 };
+
+/**
+ * Check if block is page block.
+ *
+ * @param {string} clientId Block client ID.
+ * @return {boolean} Boolean if block is / is not a page block.
+ */
+export const isPageBlock = ( clientId ) => {
+	const block = getBlock( clientId );
+	return block && 'amp/amp-story-page' === block.name;
+};