Use format-library for the formatting bar

[koke]

This PR is here to help test the changes in gutenberg: WordPress/gutenberg#12249

Requires changes in Aztec: wordpress-mobile/AztecEditor-Android#777

Requires RN Aztec changes wordpress-mobile/react-native-aztec#105

Testing Instructions (for Android)

• Apply this patch
• Apply the following patch: https://gist.github.com/Tug/50f0a51b7833a94d5f05f4fa7020a289
  You can use the following command for this:

curl "https://gist.githubusercontent.com/Tug/50f0a51b7833a94d5f05f4fa7020a289/raw/3317f9ab4dd096cb2a358c44bb5e1085a8a15550/debug-with-aztec.patch" | git apply -v

• Have https://github.com/wordpress-mobile/AztecEditor-Android in the same directory as gutenberg-mobile and checkout this patch
• Run yarn android or launch with Android Studio

Testing Instructions (for iOS)

• Apply this patch
• Run yarn ios or launch with XCode

[hypest]

I tried this on Android and (besides the fact that I had to temporarily comment out the moduleDidMount call which is not available on Android) it seems that the selected word is not picked up as a whole. See the following gif: (the r is missing from the selected word)

[Tug]

@hypest Thanks for reviewing. Strangely, I'm unable to reproduce this issue. Could it be possible that you tested while I was rebasing the gutenberg PR about an hour ago?
Edit: Wait, I'm able to reproduce now! Investigating...

[hypest]

I can try again now just to be sure and will report back 👍

[hypest]

Still happening for me on 08e345a. Using a Pixel 2 XL, Android 9.

[koke]

Those buttons look really odd there, is that the final design?

[Tug]

@hypest Thanks! I thought I had fixed it with WordPress/gutenberg@f7513a8 but it seems it only works in Aztec iOS... I just added a hack to fix the problem. I think it's good enough for our need for now, but we might want to revisit later.

Those buttons look really odd there, is that the final design?

I think they should be good enough for this first version, I used Button from react-native so they get the platform's styling (and they don't look too bad on iOS to be honest). I explored using Button from @wordpress/components but it seems the styles were hardcoded there so they look like square icon buttons (aspectRation: 1...). It will require a whole different set of changes so they work for both the Block toolbar and for those modal buttons.

[koke]

@iamthomasbishop what do you think? If there's no way we can make those look more like the designs, I think it might be better to at least drop the colors and keep them both white

[hypest]

I can confirm that latest fixes have removed the missing character issue @Tug 👍.

[hypest]

Can you also update this PR from master @Tug ? There's an annoying issue where on Android the app red-screens on start because it tries to contact the bridge, which is already fixed in master.

[Tug]

@hypest Sure, it's done now :)

[hypest]

@Tug, can you verify that Bold/Italic/Strikethrough are working for you in the paragraph block (possibly in other too) when just toggling the format without anything selected? When I try it on Android I don't see the formats "stick".

Steps:

1. Focus a paragraph block with some text already in it
2. Tap on a word but don't select any part of it
3. Tap on the "Bold" format button on the toolbar
4. Notice the "Undo" button becoming active but no other visual change happening, including no "pressed" state for the Bold button
5. Type in some characters. Notice that the new characters are not in bold 💥

[Tug]

@koke @iamthomasbishop Updated the Buttons to use a custom version instead (will need to be refactored in @wordpress/components later) using TouchableOpacity, View and Text. It seems to work pretty well's indeed an improvement on android imo:

@hypest Haven't tested Android much tbh. Looking now 👍

[koke]

I removed my previous comment since I hadn't actually pulled the latest changes correctly 🤦‍♂️

[koke]

Tested on iPhone 8, and the UI is not visible somehow

[iamthomasbishop]

Sorry for the delay! This is looking pretty dang solid for the quick first iteration! Well done. A couple immediate thoughts to get this shipped and in test hands:

• The border-radii of bottom-left and bottom-right should be 0.
• Can we round off the left/right sides of the drag handle to be fully rounded?
• I believe the margin between the top of the sheet and the drag handle should be 5pt
• What size are the table cell labels/values? I think they should probably be 16 to be safe
• What is the plan for Link to Existing Content function, as that was recently added and is a very highly requested feature – I'd hate to lose that

For reference, here's kinda where the designs were at for links between platforms, but I'm not necessarily expecting us to get there immediately:

Thanks!

[koke]

I've changed the target branch to develop, which is right now the same commit as master, so there shouldn't be any issue

[Tug]

@danielebogo Right I did notice those problems as well at different points but didn't manage to find the cause except for the fact that, due to a sequence of events, Aztec and gutenberg-mobile gets de-synchronized and that one editor does not convert to the the same html as the other.

@hypest @koke Imo it's still not ready to merge, I'd like to spend some more time on it and see if we can mitigate those issues and at least fix the rendering loop problem.

[koke]

😢 The rendering loop sounds like a merge blocker, I think the other two we could solve incrementally on a separate PR

[Tug]

Another thing: it seems AztecEditor-iOS returns <strike> tags instead of <del> in some cases. It causes inconsistencies in the selected formats on iOS.

[koke]

I found a couple issues:

• On Android, toggling the format for a selection works, but it unselects the text. This is a minor annoyance and we can deal with this later.
• On iOS (iPhone 8), tapping on the link button seems to present the bottom sheet but it's not visible, the screen just darkens:
  

[hypest]

Just mentioning it here for tracking it: there's a known issue: "b" tags are not recognised by the format-library as "bold" so, the toolbar doesn't highlight the "B" button when the cursor is on a <b></b> section. That behavior is same on GB-web.

[koke]

Another thing I noticed on iOS is that when the link settings come up, the block list behind it scrolls up and the block is no longer visible:

[iamthomasbishop]

A note on the styling of this sheet as we work towards shipping it – the styling of the Sheet component has been changed/consolidated, so we'll need to update the styling slightly. I understand we're getting close to shipping so I'm hoping this doesn't add much work – it'll just feel odd relative to the other updated sheets (Inserter in-progress, Image Settings done, Media stuff done).

Here's what the updated "neutral" Sheet looks like:

(iOS left, Android right)

The notable differences:

• Sheet Title is removed *
• Top Bar actions are removed – remove link is added as a button in the Tableview, and user can dismiss by swiping down or tapping the scrim.
• We are using a new "neutral" Table style. The only diffs between platforms are in font-family (SF Pro Text on iOS, Roboto on Android), and row indentation (w/ icons 56pt on iOS, 72dp on Android).

* – The plan is to do some usability testing to see if we need to show an explicit title for clarity, but this aligns with the majority of examples we referenced during discovery.

Might be worth coordinating with @etoledom and @marecar3 to figure out how they've been styling/re-using the component.

[daniloercoli]

I think the other two we could solve incrementally on a separate PR

Just ended another round of testing and the rendering loop issue is gone. Although I've found the problem with toolbar/styling pretty annoying. Once it happens you cannot go back to plain text. Even if you switch to another block , and then back to the "problematic" one, the issue is still there.
At the end of the video you will see me trying to remove styling without luck.

https://cloudup.com/cXNXc52YxKK

I've also tried by jumping to another block and back, with no luck.

[hypest]

Here's a quick list of the issues that still seem outstanding (will create tickets later):

1. [Android] Word gets "bold": Start with two words text like "two words". Place the cursor between the space and the "w". Tap on "B" to turn on bold. Type a few chars and press "space". Notice the "word" becoming strong while it shouldn't.
2. [iOS] Style doesn't apply: Start with "two words". Place the cursor between the space and the "w". Tap on "B" to turn on bold. Type a few chars and notice that only the first new char is in bold. The rest are not styled.
3. [Android] Selection is dropped. Select a word and style it (example: to bold) by tapping on a toolbar button. Word gets stlyed but selection is removed.
4. Haven't replicated this but as per Tug: it seems AztecEditor-iOS returns <strike> tags instead of <del> in some cases. It causes inconsistencies in the selected formats on iOS.
5. [Android] Known issue: "b" tags are not recognized by the format-library as "bold" so, the toolbar doesn't highlight the "B" button when the cursor is on a <b></b> section. That behavior is same on GB-web but I think it's counter-intuitive.
   5b. [iOS] on iOS, the editor mutates the markup, replacing <b> with <strong>s. Issue it that this doesn't align with what Gutenberg-web does.
6. [Both platforms] Styling issues as reported by Thomas above.
7. [Android] Style button gets highlighted again. See video reported by @daniloercoli : https://cloudup.com/cXNXc52YxKK
8. [Android] Note for the next pass/fixes: setting the last content from the "onSelectionChange" event feels wrong and we should remove it. Makes the logic more complex and it may introduce Aztec/RN sync bugs.

Will now update the PRs and start the merge domino since we haven't deemed any of the issues as blockers for merging this and iterating on follow up PRs. cc @koke

[koke]

[iOS] Style doesn't apply: Start with "two words". Place the cursor between the space and the "w". Tap on "B" to turn on bold. Type a few chars and notice that only the first new char is in bold. The rest are not styled.

Tested on 12.1 and it seems to work fine 🎉