Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upFixed quote<->list transformations #3525
Conversation
jorgefilipecosta
self-assigned this
Nov 16, 2017
jorgefilipecosta
requested review from
aduth,
youknowriad and
gziolo
Nov 16, 2017
gziolo
requested changes
Nov 17, 2017
Code looks good and works perfectly fine for two items. However, whenever you have more than 2 items on the list or more than two paragraphs in the quote, they are removed when transforming. See screencast:
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
codecov
Nov 17, 2017
Codecov Report
Merging #3525 into master will increase coverage by
0.95%.
The diff coverage is0%.
@@ Coverage Diff @@
## master #3525 +/- ##
=========================================
+ Coverage 34.85% 35.8% +0.95%
=========================================
Files 261 262 +1
Lines 6717 6862 +145
Branches 1225 1274 +49
=========================================
+ Hits 2341 2457 +116
- Misses 3691 3716 +25
- Partials 685 689 +4| Impacted Files | Coverage Δ | |
|---|---|---|
| blocks/library/list/index.js | 6.89% <0%> (ø) |
|
| blocks/library/shortcode/index.js | 33.33% <0%> (-6.67%) |
|
| blocks/library/html/index.js | 14.28% <0%> (-2.39%) |
|
| blocks/library/more/index.js | 20% <0%> (-2.23%) |
|
| editor/store-persist.js | 100% <0%> (ø) |
|
| blocks/hooks/index.js | 100% <0%> (ø) |
|
| ...or/components/block-inspector/advanced-controls.js | 0% <0%> (ø) |
|
| blocks/api/parser.js | 98.48% <0%> (+0.48%) |
|
| blocks/api/serializer.js | 98.85% <0%> (+0.66%) |
|
| blocks/hooks/anchor.js | 80.76% <0%> (+0.76%) |
|
| ... and 5 more |
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 5c93d78...3bf5a17. Read the comment docs.
codecov
commented
Nov 17, 2017
•
Codecov Report
@@ Coverage Diff @@
## master #3525 +/- ##
=========================================
+ Coverage 34.85% 35.8% +0.95%
=========================================
Files 261 262 +1
Lines 6717 6862 +145
Branches 1225 1274 +49
=========================================
+ Hits 2341 2457 +116
- Misses 3691 3716 +25
- Partials 685 689 +4
Continue to review full report at Codecov.
|
| value: [ <p key="list">{ toBrDelimitedContent( values ) }</p> ], | ||
| value: ( values.length === 1 ? values : initial( values ) ) | ||
| .map( ( value ) => ( { children: <p> { get( value, 'props.children' ) } </p> } ) ), | ||
| citation: ( values.length === 1 ? undefined : get( last( values ), 'props.children' ) ), |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
It works perfectly fine now. |
jorgefilipecosta
merged commit 804ab1e
into
master
Nov 17, 2017
jorgefilipecosta
deleted the
fix/fixed-quote-list-transformations
branch
Nov 17, 2017
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
Thank you for your review @gziolo. |
jorgefilipecosta commentedNov 16, 2017
•
edited
Description
This PR aims to fix a bug that I just discovered. Transforming from a quote into a list throws an error, and transforming a list into a quote creates a quote with empty content.
This PR solves both problems and simplifies the logic in a similar way of what was done in paragraph<->list transformations.
Now, transforming from quote to list, creates a list of the quote paragraphs and the last item being the citation. Transforming a list into a quote creates a paragraph for each list item excluding the last one that is used as a citation. This makes the transformations (quote->list->quote, list->quote->list)
totally idempotent.
How Has This Been Tested?
Transform a quote (empty and non-empty) into a list. Verify the first two values of the list are quote, citation.
Transform a list (empty and non-empty) into a quote. Verify the first two values of the list are now quote and citation of the quote block. If the list contains more than two values just the first two should be used.
Screenshots (jpeg or gifs if applicable):
Before:
List to quote
Quote to list
After: