Registers the core/group Block as the default block for handling "grouping" interactions #15774
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
getdave
requested review from
aduth,
ajitbohra,
chrisvanpatten,
ellatrix,
gziolo,
jorgefilipecosta,
mkaz,
notnownikki,
Soean,
talldan and
youknowriad
as code owners
|
This all looks good. It would be great to add simple unit tests which will cover new selector and reducer. |
gziolo
added
[Feature] Block API
getdave
added a commit
that referenced
this pull request
May 31, 2019
Note these test will need updating when we land #15774
getdave
added a commit
that referenced
this pull request
Jun 3, 2019
Note these test will need updating when we land #15774
getdave
added a commit
that referenced
this pull request
Jun 5, 2019
* Experiment with rudity mentory mechanic to allow Block Grouping Tried a few approaches via `insertBlock/removeBlocks` and even `replaceBlocks` but nothing preserved the undo history apart from this rather brute force method. * Migrate to `core/group` due to renaming of container Block from `core/section` * Adds conditionals to hide Group btn if selection is only a single `core/group` Block * Adds transform and updates Group button implementation to mirror Adds a `from` transform to the `core/group` Block. Currently this only works for `core/paragraph` but will need to work for all Block types. Updates the convert button to utilise `switchToBlockType` which invokes the same functionality as used in Block transform thereby unifiying the two methods of grouping. * Adds and applies Group icon As provided here #14908 (comment) * Adds editor shortcut for Grouping Blocks * Add test to check for blocks before attempting replace * Adds wildcard Block transforms A major update to the transforms logic to enable wildcard Block transforms. * Pass Block names into transform callback function - enables dynamic Block creation in wildcard transforms (see `core/group`) * Add edge cases to Block transformation logic to account for specifying `*` (all) Block types as valid for a transformation * Remove unwanted test that checks all Blocks are of the same type before allowing a transform on a Multi Block selection * Reinstate missing createBlock dep after rebase * Fix to avoid allowing single Group Block to be Grouped * Update to use more appropriate logical negation operator for comparison Addresses #14908 (comment) * Extracts key transform test logic into dedicated methods Previously hard coded values and difficult to follow logic were making the `switchToBlockType` overly complex. Extracted to well name methods to improve readability and allow to further refactoring. * Extract method to test for wildcard block transforms DRYs up code by extracting a function to test for the presence of a wildcard block transform * Moves logic to allow for wildcard transform into central transform checking method Previously test to allow wildcard transform to be valid were manually added as edge cases in conditions in predicate functions. This update centralises all logic to test whether a given transform is possible but including the logic that allows wildcard transforms within the main method `isPossibleTransformForSource` which determines whether a given transform is possible. * Adds UnGrouping mechanic * Adds UnGroup Icon * Adds e2e test to cover basic grouping for single and multiple blocks * Fix edge case with test to detect a single group container Block * Adds UnGroup keyboard shortcut * Adds more e2e test coverage Includes testing grouping via transforms, options menu and keyboard shortcuts * Adds check for group block availability before displaying grouping UI Also adds e2e tests to cover this. * Updates misnamned components Addresses #14908 (comment) * Updates to preserve widest width alignment of child block on group container Previously if one of the child blocks being grouped had a width alignment (eg: wide or full) then the group block did not respect this. This meant that layouts weren’t preserved when grouping. Adds functionality and tests to ensure that when a group is created the widest alignment setting of the child blocks is set on the group block. * Updates to DRY out tests * Updates to simplify test setup Previously API calls were cleaning up blocks. This can be removed because all posts are auto removed before each test is run. Addresses #14908 (comment) * Updates to simplify test assertion Addresses #14908 (comment) * Combines test cases to simplify and reduce number of test required Addresses #14908 (comment) * Updates to combine test for grouping and ungrouping via options menu Addresses #14908 (comment) * Adds keyboard shortcut to global keyboard shortcuts modal * Ensure correct case for group shortcut (ie: lower) * Add shortcut to Block settings menu dropdown items * Adds translation context to Group actions in menus Addresses #14908 (comment) * Update Block Transforms fixtures to show all Blocks can transform to Group Block * Updates Keyboard Shortcut test snapshot to include Group/UnGroup * Fix to ensure Group block accounted for * Fix Block deletion test failure via keyboard workaround Due to the addition of the “Group” item into the Block Options toolbar the “Remove Block” item had been removed outside the viewable portion of the Popover (not the popover has a restricted height and allows scrolling to see the additional items if there is insufficient space to display them all). Pupeteer isn’t able to click on items that are not visible. The simplest work around is to us the keyboard to select the “Remove Block” menu item rather. Trying to scroll a div within Pupeteer is fraught with problems and inconsistencies. * Fixes Remove Block button helper to be more resilient to change Previously we relied on the number of tab stops to locate the correct button in the menu. This change uses the actual text of the button to identify it and uses an assertion to ensure that the correct button is explicitly required. * Rename function to better convey intent Addresses #14908 (comment) * Fixes to catch transforms which are invalid for blocks of different types A check was accidentally removed in `b2af0f2611e2a2bc66c62959dbf483abcbe103a9` which allowed multiple blocks of different types to be considered valid even if they were not. Adds conditional to ensure that unless the transform is a wildcard then we test that all the blocks are of the same type as the first block’s type before considering the transform to be valid. * Removes redundant snapshots * Fixes Transforms test to not over-test Group transforms Previously we tested every block transforming into the Group Block. This was unwanted overhead. Fixed to only test 1 single Block transforming into Group. Removed redundant snapshots as a result of removing it. * Fixes e2e test by reinstating helper lost during rebase * Fix to make Group transform snapshot tests more resilient to change Now explicity selects paragraph and image blocks to test the Group transform rather than simply testing the first block that transform into a Group. This ensures that if the order of transforms changes in the fixtures the test won’t be accidentally invalidated. Resolves: #14908 * Updates to DRY out test and reduce test redundancy and performance Through use of smarter filtering we can get away with a single `it.each()` block which improves perf and removes redundant tests. Addresses #14908 (comment) * Adds unit tests to cover new conditionals added for wildcard blocks transforms * Fixes capitalisation of UnGroup to Ungroup Resolves #14908 (comment) * Updates doc block to reflect possible nullable return type Addresses #14908 (comment) * Updates Readme with doc update * Updates to remove unwanted comments * Updates capitalisatoin of “wildcard” Addresses #14908 (comment) * Update comment with more context for future maintainers Addresses #14908 (comment) * Updates to remove unwanted level of context on the translation Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com> * Adds tests to cover isWildcardBlockTransform * Adds tests for isContainerGroupBlock function Note these test will need updating when we land #15774 * Fixes logic around multi blocks of same type and adds tests Prevously we had 1 function attempting to test for multiple block selection and checking that the selection was all of the same block type. This caused bugs within `switchToBlockType` because the logic was confusing. For example, if a selection isn’t a multi block then we don’t need to test that all the blocks are the same. Separated the two functions and updated conditions in switchToBlockType to reflect this. Added unit tests to cover two new functions. * Adds new generator based API signature to Block transforms Previously the transforms function was pased two arguments 1. attributes 2. innerblocks This wasn’t extensible and more advanced transformations require more information about the block (eg: name). To avoid bloating the signature, a progressive enhancement approach is applied whereby if a generator function is passed as the transform then we pass the entire block object to the generator. This is opt-in only and is backwards compatible with all existing transform functions. * Adds new apply option method to transform API Previously we were modifying the existing transform function to conform to the requirements of a new API (ie: receiving the entire block object rather than the individual arguments). It was decided that introducing a new `apply` option and soft deprecating the old transform option would be preferable. The apply option if provided now takes precedence over the transform option. This is fully backwards compatible. See https://wordpress.slack.com/archives/C02QB2JS7/p1559567845087000 * Updates Block Reg docs to cover wildcard transforms * Updates changelog to document wildcards and transform apply option * Fix linting error introduce in rebase * Fixes test util to avoid infinite loops Previously if the button wasn’t found then the loop would continue forever looking for the button. This would have caused timeouts. Limits the loop to the number of buttons in the document. Also bails out immediately having found the button. Resolves #14908 (comment) * Renames apply to convert to avoid confusion with Func.apply To avoid potential confusion and overlap with Function.apply, rename to `convert`. Slack discussion: https://wordpress.slack.com/archives/C02QB2JS7/p1559593099150300?thread_ts=1559571243.134500&cid=C02QB2JS7 MDN Function apply docs: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply Resolves: #14908 (comment) * Fixes unecessary additional var introduced during debugging * Fix convert API to match established patterns for consistency Previously `convert` always passed an array of block objects even if there was only a single block. This is inconsistent with the implementation of the existing `transform` method which passes only a block’s attributes/innerBlocks pair when it is not a multi block. To retain consistency with the existing `isMultiBlock` paradiagm this updates the API of `convert` to pass a single block object when not in multiblock mode. * Fixes error in docs Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com> * Fixes doc blocks to match coding style Resolves #14908 (comment) * Fixes icon size and color in dropdown for Group/Ungroup * Updates to remove keyboard shortcuts Following a discussion it was not possible to achieve a consensus on which shortcuts was most suitable (or indeed whether keyboard shortcuts for this were even a good idea). As a result this has been descoped from this PR and will be addressed elsewhere. Once merged I will push a new placeholder PR with the foundation for shortcuts in place and others can then amend it. * Updates snapshot to account for removing keyboard shortcuts * Removes e2e tests covering keyboard shortcuts * Fixes unwanted check introduced during debugging Test for existence of transform is not required and was introduced during debugging. Can be removed. * Updates to collapse spaces in doc blocks Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com> * Fixes isWildcardBlockTransform to test for Array-ness Resolves #14908 (comment) * Fixes incorrect capitalisation of “UnGroup” to “Ungroup” Addresses #14908 (comment) * Updates to remove redundant isMultiBlockSelection util function Addresses #14908 (comment) * Reinstate missing Ungroup icon Accidentally got lost during renaming of “UnGroup” to “Ungroup”!
Enables the ability register a given block as the block which handles “grouping” interactions within the editor. Related #14908
getdave
force-pushed
the
add/register-grouping-block-with-store
branch
from
b08696c
to
0460751
Compare
|
@gziolo How's this looking now? |
|
Travis has some concerns. I guess https://travis-ci.com/WordPress/gutenberg/jobs/208548466#L531 |
gziolo
approved these changes
Jun 18, 2019
This was referenced Jun 25, 2019
aduth
reviewed
Jun 25, 2019
| @@ -135,4 +136,8 @@ export const registerCoreBlocks = () => { | |||
| setFreeformContentHandlerName( classic.name ); | |||
| } | |||
| setUnregisteredTypeHandlerName( missing.name ); | |||
|
|
|||
| if ( group ) { | |||
There was a problem hiding this comment.
I was browsing the file and came to ask the same thing. :) @getdave
|
This (with #16278) seems like a good improvement to avoid hard-coding and for consistency with the default and unknown block name handling 👍 |
|
Noting here that as this stands in Gutenberg 6.0.0 it's not yet possible to have the newly registered Group Block automatically be used for "Grouping" interactions. That is being handled in #16278 |
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds the ability to assign the
core/groupblock as the block used in "grouping" interactions which will be introduced in #14908.Please see the related PR for context. Once the Grouping PR is shipped then (if acceptable) I will look to land this PR. Lastly, in a final PR I will update the logic around grouping interactions to utilise the block registered for grouping.
How has this been tested?
Needs a test plan...
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: