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

Block: Assign block list ref from block child #3205

Merged
merged 1 commit into from
Oct 27, 2017
Merged

Conversation

aduth
Copy link
Member

@aduth aduth commented Oct 27, 2017

Regression introduced in #3171

This pull request is a partial revert of #3171, seeking to address an error which occurs when removing a block. The changes in #3171 depend to access props of a rendered element ref are insufficient, since the ref callback is invoked with null when the element unmounts. The changes here partially restore the original behavior prior to #3171, with an optimization to still use a constant reference to binding the block ref in VisualEditorBlockList by passing UID from the child Block component's own ref callback.

Testing instructions:

Verify that no errors occur when removing a block.

@aduth aduth added the [Feature] Blocks Overall functionality of blocks label Oct 27, 2017
@aduth aduth requested a review from youknowriad October 27, 2017 14:18
@youknowriad
Copy link
Contributor

Interestingly, this bug has been catched by the e2e tests when I rebased #3069

@codecov
Copy link

codecov bot commented Oct 27, 2017

Codecov Report

Merging #3205 into master will decrease coverage by 0.34%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #3205      +/-   ##
=========================================
- Coverage   31.54%   31.2%   -0.35%     
=========================================
  Files         221     221              
  Lines        6311    6381      +70     
  Branches     1121    1148      +27     
=========================================
  Hits         1991    1991              
- Misses       3632    3675      +43     
- Partials      688     715      +27
Impacted Files Coverage Δ
editor/modes/visual-editor/block.js 0% <0%> (ø) ⬆️
editor/modes/visual-editor/block-list.js 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update efa08a0...01c2189. Read the comment docs.

@aduth aduth merged commit 48655db into master Oct 27, 2017
@aduth aduth deleted the fix/block-remove-error branch October 27, 2017 14:43
@mtias mtias mentioned this pull request Oct 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants