Skip to content

Quote: Fix new quote, transform from list block#2349

Merged
aduth merged 4 commits intomasterfrom
fix/quote-value
Aug 11, 2017
Merged

Quote: Fix new quote, transform from list block#2349
aduth merged 4 commits intomasterfrom
fix/quote-value

Conversation

@aduth
Copy link
Copy Markdown
Member

@aduth aduth commented Aug 10, 2017

Fixes #2343

This pull request seeks to resolve issues relating to quote initialization and list transforms.

The quote block does not define a default value but assumes it to be an array, iterating by Array#map in its save implementation. While we could add a default value to the attributes definition, the approach here instead drops the Array#map since, for all I could discern, it serves no purpose (generates identical output). Instead, the value is rendered verbatim.

Further, there were some issues with list transforms, particularly for lists which do not contain any special formatting or list nesting. In these cases, list items would be plain strings, but the block's internal toBrDelimitedContent utility was not prepared to handle these cases.

Lastly, the List -> Quote transform was not transforming into the expected value type of quote and should have been wrapped in the expected array of paragraphs.

Testing instructions:

Verify that you can create a new Quote block.

Verify that you can convert from List block to Quote (and Paragraph) block with:

  • No content
  • Single list item (with and without formatted content)
  • Multiple list items (with and without formatted content)
  • Nested list items (with and without formatted content)

aduth added 3 commits August 10, 2017 10:10
Should have same result, unless we were explicitly trying to strip props from root p node, in which case this is not assigned in quote block implementation, and should be the responsibility of the transform source to remove in transform value
@aduth aduth added the [Feature] Blocks Overall functionality of blocks label Aug 10, 2017
@aduth
Copy link
Copy Markdown
Member Author

aduth commented Aug 10, 2017

Oh failing tests, you illuminate why we'd done the map, though really shouldn't be necessary facebook/react#7038.

Needed to ensure key is assigned into map result to avoid React error. Seems unnecessary since we don't need the diffing reconciliation for generating static markup, but alas.

See: facebook/react#7038
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 10, 2017

Codecov Report

Merging #2349 into master will increase coverage by 0.01%.
The diff coverage is 20%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2349      +/-   ##
=========================================
+ Coverage   24.69%   24.7%   +0.01%     
=========================================
  Files         152     152              
  Lines        4738    4962     +224     
  Branches      799     849      +50     
=========================================
+ Hits         1170    1226      +56     
- Misses       3014    3136     +122     
- Partials      554     600      +46
Impacted Files Coverage Δ
blocks/library/list/index.js 6.55% <0%> (-0.68%) ⬇️
blocks/library/quote/index.js 14.28% <100%> (ø) ⬆️
blocks/library/separator/index.js 42.85% <0%> (-7.15%) ⬇️
blocks/library/cover-text/index.js 29.41% <0%> (-5.59%) ⬇️
blocks/library/heading/index.js 18.91% <0%> (-4.9%) ⬇️
blocks/library/latest-posts/index.js 7.22% <0%> (-2.78%) ⬇️
blocks/library/gallery/index.js 23.8% <0%> (-1.2%) ⬇️
blocks/library/image/index.js 13.95% <0%> (+1.7%) ⬆️
blocks/library/embed/index.js 49.37% <0%> (+3.92%) ⬆️

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 31879c2...f1952ac. Read the comment docs.

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.

Editor crashes to white page when toggling list to quote

1 participant