Enable the verse block for mobile #21883

guarani · 2020-04-24T21:59:19Z

Addresses: wordpress-mobile/gutenberg-mobile#1886
Related: wordpress-mobile/gutenberg-mobile#2185

Description

Enable the verse block for production builds.

How has this been tested?

Download the test build for iOS: Create test build for verse block wordpress-mobile/WordPress-iOS#14004
Perform the test cases for the Verse block from Create test cases for new verse block wordpress-mobile/test-cases#28
- TC001: Tapping return key creates a new line within the block
- TC002: Setting text alignment works as expected
- TC003: Whitespace is preserved when applying formatting options (bold, italics, etc)
- TC004: The verse block can be merged with the paragraph block
Create and publish a verse block on mobile and ensure it's viewable on the web and on the user's site itself
Create and publish a verse block on the web and ensure it appears in the app
Repeat steps 1 to 4 for the test build for Android: Create test build for verse block wordpress-mobile/WordPress-Android#11763

Demo

	iOS	Android
Preview

Types of changes

New feature: verse block for mobile

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

github-actions · 2020-04-24T22:05:01Z

Size Change: +10 B (0%)

Total Size: 824 kB

Filename	Size	Change
`build/block-library/index.js`	115 kB	+10 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.02 kB	0 B
`build/annotations/index.js`	3.62 kB	0 B
`build/api-fetch/index.js`	4.08 kB	0 B
`build/autop/index.js`	2.82 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/index.js`	6.61 kB	0 B
`build/block-directory/style-rtl.css`	760 B	0 B
`build/block-directory/style.css`	761 B	0 B
`build/block-editor/index.js`	102 kB	0 B
`build/block-editor/style-rtl.css`	10.3 kB	0 B
`build/block-editor/style.css`	10.3 kB	0 B
`build/block-library/editor-rtl.css`	7.12 kB	0 B
`build/block-library/editor.css`	7.12 kB	0 B
`build/block-library/style-rtl.css`	7.34 kB	0 B
`build/block-library/style.css`	7.34 kB	0 B
`build/block-library/theme-rtl.css`	683 B	0 B
`build/block-library/theme.css`	685 B	0 B
`build/block-serialization-default-parser/index.js`	1.88 kB	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/blocks/index.js`	48.1 kB	0 B
`build/components/index.js`	180 kB	0 B
`build/components/style-rtl.css`	17 kB	0 B
`build/components/style.css`	16.9 kB	0 B
`build/compose/index.js`	6.66 kB	0 B
`build/core-data/index.js`	11.4 kB	0 B
`build/data-controls/index.js`	1.29 kB	0 B
`build/data/index.js`	8.45 kB	0 B
`build/date/index.js`	5.47 kB	0 B
`build/deprecated/index.js`	772 B	0 B
`build/dom-ready/index.js`	568 B	0 B
`build/dom/index.js`	3.1 kB	0 B
`build/edit-navigation/index.js`	4.41 kB	0 B
`build/edit-navigation/style-rtl.css`	618 B	0 B
`build/edit-navigation/style.css`	617 B	0 B
`build/edit-post/index.js`	28 kB	0 B
`build/edit-post/style-rtl.css`	12.2 kB	0 B
`build/edit-post/style.css`	12.2 kB	0 B
`build/edit-site/index.js`	12.1 kB	0 B
`build/edit-site/style-rtl.css`	5.22 kB	0 B
`build/edit-site/style.css`	5.22 kB	0 B
`build/edit-widgets/index.js`	8.37 kB	0 B
`build/edit-widgets/style-rtl.css`	4.69 kB	0 B
`build/edit-widgets/style.css`	4.69 kB	0 B
`build/editor/editor-styles-rtl.css`	425 B	0 B
`build/editor/editor-styles.css`	428 B	0 B
`build/editor/index.js`	44.3 kB	0 B
`build/editor/style-rtl.css`	5.07 kB	0 B
`build/editor/style.css`	5.08 kB	0 B
`build/element/index.js`	4.65 kB	0 B
`build/escape-html/index.js`	734 B	0 B
`build/format-library/index.js`	7.63 kB	0 B
`build/format-library/style-rtl.css`	502 B	0 B
`build/format-library/style.css`	502 B	0 B
`build/hooks/index.js`	2.14 kB	0 B
`build/html-entities/index.js`	622 B	0 B
`build/i18n/index.js`	3.56 kB	0 B
`build/is-shallow-equal/index.js`	710 B	0 B
`build/keyboard-shortcuts/index.js`	2.51 kB	0 B
`build/keycodes/index.js`	1.94 kB	0 B
`build/list-reusable-blocks/index.js`	3.12 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	226 B	0 B
`build/list-reusable-blocks/style.css`	226 B	0 B
`build/media-utils/index.js`	5.29 kB	0 B
`build/notices/index.js`	1.79 kB	0 B
`build/nux/index.js`	3.4 kB	0 B
`build/nux/style-rtl.css`	616 B	0 B
`build/nux/style.css`	613 B	0 B
`build/plugins/index.js`	2.56 kB	0 B
`build/primitives/index.js`	1.5 kB	0 B
`build/priority-queue/index.js`	789 B	0 B
`build/redux-routine/index.js`	2.85 kB	0 B
`build/rich-text/index.js`	14.8 kB	0 B
`build/server-side-render/index.js`	2.68 kB	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.28 kB	0 B
`build/url/index.js`	4.02 kB	0 B
`build/viewport/index.js`	1.84 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.18 kB	0 B

_{compressed-size-action}

guarani · 2020-04-24T22:05:22Z

packages/block-library/src/verse/edit.js

@@ -34,6 +34,7 @@ export default function VerseEdit( {
 			</BlockControls>
 			<RichText
 				tagName={ Block.pre }
+				identifier='content'


It looks like this is a required attribute to allow the verse block to be merged with the paragraph block, otherwise I would get a console error:

gutenberg/packages/block-editor/src/store/effects.js

Line 103 in 4bbfbc1

`RichText needs an identifier prop that is the block attribute key of the attribute it controls. Its type is expected to be a string, but was ${ typeof attributeKey }`

I essentially copied this from the paragraph block:

gutenberg/packages/block-library/src/paragraph/edit.native.js

Line 39 in 4bbfbc1

identifier="content"

guarani · 2020-04-27T02:10:53Z

~~I noticed on the last build that the bug from wordpress-mobile/gutenberg-mobile#1966 is appearing again and I'll need to investigate, so leaving this as a draft.~~
It was just an error with my test setup, fixed now.

guarani · 2020-04-28T00:28:28Z

When return is tapped inside a verse block on the mobile editor, a   tag is inserted – if the same happens on web, a newline character is inserted.
The post list screen on Android and iOS handle   tags differently, they are rendered correctly on Android but are not rendered on iOS. (Both platforms handle newline characters correctly.)

Android	iOS

This issue is not specific to the verse block – the preformatted block already has this issue – it uses   tags for on mobile but use newline characters on web.

I propose:

Not treating this as a blocker for releasing the verse block.
Adding a subissue to the verse block master issue to make the post list screen more resilient on iOS by adding support for rending newline characters. This should fix all preexisting posts that have this issue (posts with preformatted blocks created on mobile) as well as posts with verse blocks once it is released.
Investigate whether new lines should be treated the same on mobile and web for the preformatted and verse blocks, and if possible, unify behavior.

Any thoughts @iamthomasbishop?
cc @hypest

hypest · 2020-04-28T09:17:33Z

The post list screen on Android and iOS handle
tags differently...

Is this also happening on posts written in the classic editor (Aztec) and on posts written on the web?

Also, is this happening just for PRE-based blocks/markup or the different newline/
behavior between the platforms also happens in normal paragraphs? Trying to understand if this is a block editor vs classic editor issue. Or even specific-block vs other blocks issue.

It looks like an issue with the posts list rendering itself, in which case I wouldn't consider it a blocker for the Verse block.

Adding a subissue to the verse block master issue...

Yes please, thanks!

Investigate whether new lines should be treated the same on mobile and web for the preformatted and verse blocks, and if possible, unify behavior.

I'd add that as another subtask.

guarani · 2020-04-28T15:18:01Z

Is this also happening on posts written in the classic editor (Aztec) and on posts written on the web?

This is not happening on posts created in the classic editor nor posts written on the web.

Also, is this happening just for PRE-based blocks/markup or the different newline/
behavior between the platforms also happens in normal paragraphs?

Tapping return inside a paragraph block just creates a new block, which I think makes paragraphs a different category – do you agree?

Added issues here:

Use newlines characters instead of tags for new lines in pre-based blocks wordpress-mobile/gutenberg-mobile#2193 Use newlines characters instead of
tags for new lines in pre-based blocks
Render tags as new lines in post list post preview wordpress-mobile/WordPress-iOS#14008 Render line breaks properly on the post list screen on iOS

etoledom · 2020-04-29T07:35:14Z

Should we add a Unit Test to this too?
I think it's specially helpful specially in Multiplatform blocks like this one.

Something simple like this should be enough:

gutenberg/packages/block-library/src/code/test/edit.native.js

Lines 13 to 20 in ea4fa88

    
           it( 'renders without crashing', () => { 
        
           	const component = renderer.create( 
        
           		<Code attributes={ { content: '' } } /> 
        
           	); 
        
           	const rendered = component.toJSON(); 
        
           	expect( rendered ).toBeTruthy(); 
        
           } );

guarani · 2020-04-29T21:49:50Z

Thanks for pointing out the lack of unit tests, @etoledom! I've added those now.

The MERGE_BLOCKS store action requires RichText components to have an identifier. The identifier must be one of the block's attributes.

Remove the devOnly flag to make the verse block not only available on debug builds, but also in production builds.

guarani · 2020-05-11T01:46:35Z

I'm getting seeing some tests fail on Travis, e.g.:

54638.11 E2E tests (Admin) (2/4)
- FAIL packages/e2e-tests/specs/editor/blocks/quote.test.js (72.091s)
  - ✕ can be merged into from a paragraph (3236ms)
54638.17 E2E tests (Author) (4/4)
- FAIL packages/e2e-tests/specs/editor/various/rtl.test.js (21.436s)
  - ✕ should navigate inline boundaries (3878ms)

I noticed the build badge was showing that Gutenberg was failing on Friday so I'm not sure if there is some flakiness in the test suite. I've re-run the test suite twice on Travis and I've also ran npm run test:e2e locally, but the tests seem to mostly fail and I haven't been able to determine the cause yet.

ceyhun · 2020-05-11T18:05:08Z

All the test cases work great on Android.
On iOS works fine as well except that I found a small issue with making text on second line bold when there's space before it:

guarani · 2020-05-11T18:19:38Z

Thanks a lot for your tests, @ceyhun.
I double checked that the work to fix the formatting issue is included:

Add option to disable collapsing of whitespaces. wordpress-mobile/AztecEditor-iOS#1277
Issue/1966 Disable whitespace collapse in Aztec wordpress-mobile/gutenberg-mobile#2127
It appears that the bug was resolved for the case where formatting is applied to the first line of text in a verse block, but on the 2nd, 3rd, etc, lines the issue is still present.

I also checked the develop branch of gutenberg-mobile and this issue is present.

@SergioEstevao I want to let you know about this because you're wrangling the release today and are familiar with this work. I'd like to see if there is still an issue with Aztec that needs to be resolved but I'm not yet very familiar with the repo.
Do you have any suggestions on a) whether we should still attempt to fix this for the code freeze, b) any pointers about what the problem might be?

guarani · 2020-05-12T22:59:41Z

The bug @ceyhun found was fixed in the Aztec library by @SergioEstevao which powers the rich text components on mobile, such as that found in the verse block. The gist of it is that the libxml2 library used by Aztec was collapsing whitespace elements when these elements were between other tags.
So in this case:

Hello<br>
   <strong>World</strong>

the whitespace between the   and the  was being stripped out. Removing the HTML_PARSE_NOBLANKS flag on libxml2's configuration fixed this! 🎉

guarani · 2020-05-12T23:08:32Z

Retest results

I found one new minor bug, and all the other test cases passed.

Minor bug found

If you type bold text, then add a few spaces, then right align the block, the spaces you typed are lost. If no formatting is applied, the bug doesn't occur. It's also iOS specific. I'd say this is another angle of the bug fixed above (cc @SergioEstevao).

@iamthomasbishop, do you agree this is not a blocker for the verse block? If so, I'll add it to the list of Verse Block non-blockers.

Here are two gifs showing the bug. Notice the extra spaces typed after the "Hello ".

As you can see, only in the first example – where the text is formatted (bolded) – is the bug present.

With formatting ❌	Without formatting ✅

guarani · 2020-05-12T23:12:36Z

@ceyhun re-requesting a review here please 🙏.

SergioEstevao · 2020-05-13T10:36:41Z

@guarani I tested with the new version of Aztec and the bug you found when switching alignment is still there.
The curious thing is that the whitespaces are kept, you can see that by switching to the HTML mode or even by typing a character.

At this point I think this is UITextView layout/presentation issue so it shouldn't be a blocker for this PR.

ceyhun · 2020-05-18T10:29:05Z

@ceyhun re-requesting a review here please 🙏.

@guarani e2e tests seem to have failed on the gutenberg-mobile PR.

SergioEstevao

All working well, extra kudos for adding the unit tests!

guarani mentioned this pull request Apr 24, 2020

Enable verse block wordpress-mobile/gutenberg-mobile#2185

Merged

2 tasks

guarani commented Apr 24, 2020

View reviewed changes

guarani changed the title ~~Enable the verse block for production~~ Enable the verse block for mobile Apr 24, 2020

This was referenced Apr 27, 2020

Create test build for verse block wordpress-mobile/WordPress-iOS#14004

Closed

Create test build for verse block wordpress-mobile/WordPress-Android#11763

Closed

guarani marked this pull request as ready for review April 28, 2020 23:45

guarani requested review from ajitbohra, jorgefilipecosta, Soean and talldan as code owners April 28, 2020 23:45

guarani requested a review from etoledom April 28, 2020 23:46

This was referenced May 4, 2020

Port Verse block to mobile wordpress-mobile/gutenberg-mobile#1886

Closed

Create test cases for new verse block wordpress-mobile/test-cases#28

Merged

guarani marked this pull request as draft May 8, 2020 20:55

guarani marked this pull request as ready for review May 8, 2020 20:56

guarani added 4 commits May 8, 2020 17:38

Set RichText identifier for verse block

8bbdd32

The MERGE_BLOCKS store action requires RichText components to have an identifier. The identifier must be one of the block's attributes.

Enable the verse block for production

faab5b5

Remove the devOnly flag to make the verse block not only available on debug builds, but also in production builds.

Change single quotes to double

c753167

Add unit test for verse block

d57347f

guarani force-pushed the rnmobile/enable-verse-block branch from 9cda988 to d57347f Compare May 8, 2020 21:38

guarani requested a review from ceyhun May 11, 2020 13:36

SergioEstevao self-requested a review May 18, 2020 20:51

SergioEstevao approved these changes May 18, 2020

View reviewed changes

guarani merged commit c2f6025 into master May 18, 2020

guarani deleted the rnmobile/enable-verse-block branch May 18, 2020 22:48

github-actions bot added this to the Gutenberg 8.2 milestone May 18, 2020

mchowning mentioned this pull request May 26, 2020

1.29.0 Release wordpress-mobile/gutenberg-mobile#2321

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable the verse block for mobile #21883

Enable the verse block for mobile #21883

guarani commented Apr 24, 2020 •

edited

Loading

github-actions bot commented Apr 24, 2020 •

edited

Loading

guarani Apr 24, 2020 •

edited

Loading

guarani commented Apr 27, 2020 •

edited

Loading

guarani commented Apr 28, 2020 •

edited

Loading

hypest commented Apr 28, 2020

guarani commented Apr 28, 2020

etoledom commented Apr 29, 2020

guarani commented Apr 29, 2020

guarani commented May 11, 2020

ceyhun commented May 11, 2020

guarani commented May 11, 2020

guarani commented May 12, 2020

guarani commented May 12, 2020 •

edited

Loading

guarani commented May 12, 2020

SergioEstevao commented May 13, 2020

ceyhun commented May 18, 2020

SergioEstevao left a comment

Enable the verse block for mobile #21883

Enable the verse block for mobile #21883

Conversation

guarani commented Apr 24, 2020 • edited Loading

Description

How has this been tested?

Demo

Types of changes

Checklist:

github-actions bot commented Apr 24, 2020 • edited Loading

guarani Apr 24, 2020 • edited Loading

Choose a reason for hiding this comment

guarani commented Apr 27, 2020 • edited Loading

guarani commented Apr 28, 2020 • edited Loading

hypest commented Apr 28, 2020

guarani commented Apr 28, 2020

etoledom commented Apr 29, 2020

guarani commented Apr 29, 2020

guarani commented May 11, 2020

ceyhun commented May 11, 2020

guarani commented May 11, 2020

guarani commented May 12, 2020

guarani commented May 12, 2020 • edited Loading

Retest results

Minor bug found

guarani commented May 12, 2020

SergioEstevao commented May 13, 2020

ceyhun commented May 18, 2020

SergioEstevao left a comment

Choose a reason for hiding this comment

guarani commented Apr 24, 2020 •

edited

Loading

github-actions bot commented Apr 24, 2020 •

edited

Loading

guarani Apr 24, 2020 •

edited

Loading

guarani commented Apr 27, 2020 •

edited

Loading

guarani commented Apr 28, 2020 •

edited

Loading

guarani commented May 12, 2020 •

edited

Loading