Skip to content

Don't allow multiple CTA / attachment block to be pasted.#3601

Merged
swissspidy merged 6 commits intodevelopfrom
fix/cta-attachment
Oct 23, 2019
Merged

Don't allow multiple CTA / attachment block to be pasted.#3601
swissspidy merged 6 commits intodevelopfrom
fix/cta-attachment

Conversation

@spacedmonkey
Copy link
Copy Markdown
Contributor

@spacedmonkey spacedmonkey commented Oct 22, 2019

Summary

Fixes #3575

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@googlebot googlebot added the cla: yes Signed the Google CLA label Oct 22, 2019
@spacedmonkey spacedmonkey requested a review from barklund October 22, 2019 20:58
@swissspidy
Copy link
Copy Markdown
Collaborator

Looks neat at first glance! 🚀

@barklund
Copy link
Copy Markdown
Contributor

Looks go to me as well (except for the lint error).

It will conflict heavily with #3600, but it's probably an easy manual merge.

* @param {string} name Name of block.
* @param {Function} createErrorNotice Bring in dispatchable createErrorNotice function.
*/
const displayPasteError = ( name, createErrorNotice ) => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having to pass createErrorNotice, couldn't this just be turned into a custom React hook?

const useDisplayPasteError = ( name ) => {
	const { createErrorNotice } = useDispatch( 'core/notices' );
	// ...
}

In assets/src/stories-editor/components/context-menu/index.js you could then do:

const displayPasteError = useDisplayPasteError( pastedBlock.name );
displayPasteError();

@swissspidy
Copy link
Copy Markdown
Collaborator

Code works well so far 👍

Just resolved the merge conflicts with #3600 now

@swissspidy swissspidy added this to the v1.4 milestone Oct 23, 2019
Copy link
Copy Markdown
Collaborator

@swissspidy swissspidy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well!

@swissspidy swissspidy marked this pull request as ready for review October 23, 2019 11:08
@swissspidy swissspidy merged commit b3149bc into develop Oct 23, 2019
@swissspidy swissspidy deleted the fix/cta-attachment branch October 23, 2019 11:08
westonruter added a commit that referenced this pull request Oct 25, 2019
…-args-debugging

* 'develop' of github.com:ampproject/amp-wp: (163 commits)
  Remove entries array test after completed.
  Update dependency core-js to v3.3.4 (#3624)
  Quick UX fixes (#3611)
  Fix most pressing RTL issues (#3558)
  Add test case for broken parent relationship
  Adapt broken test
  Throw a _doing_it_wrong() when an expected parent is not found
  Change template hierarchy query arguments to public
  Reuse data-set-focus to reliably match twentytwenty keyboard focus management
  Add tabindex attribute to modals
  Guess the role of a modal based on its classes
  Disable Code Editor (#3608)
  Update dependency postcss to v7.0.20 (#3613)
  Update dependency autoprefixer to v9.7.0 (#3614)
  Update dependency browserslist to v4.7.2 (#3612)
  Add rel=preconnect link for AMP CDN
  Fix block nav padding and margin (#3610)
  Adjust Gutenberg / WordPress requirement (#3609)
  Don't allow multiple CTA / attachment block to be pasted. (#3601)
  Fix page crashing when duplicating immovable blocks (#3593)
  ...
@spacedmonkey spacedmonkey self-assigned this Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Signed the Google CLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Page Attachment can be copied to a Page where CTA already exists

4 participants