Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inner Blocks: Refactor treatment of block settings #7418

Merged
merged 1 commit into from Jun 29, 2018

Conversation

aduth
Copy link
Member

@aduth aduth commented Jun 20, 2018

This pull request seeks to apply a few refactorings to the InnerBlocks treatment of its nested settings, namely:

  • Update deprecated componentWillReceiveProps to equivalent componentDidUpdate (per suggestion at Block List: Remove createInnerBlockList utility / context #7192 (comment))
  • Avoid deep equality check on flat allowedBlocks prop shape
  • Avoid handling unexpected case where UPDATE_BLOCK_LIST_SETTINGS is not passed an id (was never tested anyways)
  • Avoid creating new references for blockListSettings when settings not set, but the id never existed in state anyways (including new test)
  • Avoid switch fallthrough on case where previous updateIsRequired condition would be false, which could have introduced future maintainability issues if additional case statements were added
  • Update tests to realistic values
  • Add test to verify state reference is not changed when no update is needed
  • Consistently name allowedBlocks (previously also referred to as supportedBlocks)

Testing instructions:

Repeat testing instructions from #6546.

@aduth aduth added the [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P label Jun 20, 2018
@aduth
Copy link
Member Author

aduth commented Jun 20, 2018

Seeing that a few of these refactorings were also proposed as part of #6991.

@aduth aduth merged commit 124647e into master Jun 29, 2018
@aduth aduth deleted the update/inner-blocks-did-update branch June 29, 2018 14:28
@jorgefilipecosta
Copy link
Member

Hi @aduth thank you for this refactoring. I'm sorry I totally lost this PR in the middle of notifications and just noticed it now when new notifications were triggered :(
But the changes here look good 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants