Implement support for nested lists on GB-mobile.#15566
Implement support for nested lists on GB-mobile.#15566SergioEstevao merged 13 commits intomasterfrom
Conversation
# Conflicts: # packages/block-editor/src/components/rich-text/index.native.js
There was a problem hiding this comment.
Great work here @SergioEstevao
I tested both on a real android device and with my computer keyboard on an emulator and tried to reproduce the issues described in wordpress-mobile/gutenberg-mobile#874 but could not 🎉
However I noticed a bug when splitting and merging list blocks, not sure it's related to your change yet. Will investigate.
|
Here is a recording show off the issue:
|
|
Android is still not ready, the Delete on list is not being handed to the
JS code. Can you try in iOS.
…On Fri, 17 May 2019 at 13:05, Tugdual de Kerviler ***@***.***> wrote:
Here is a recording show off the issue:
[image: output]
<https://user-images.githubusercontent.com/230230/57926884-cb2ddf80-78ac-11e9-938d-ab108adaa13f.gif>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15566?email_source=notifications&email_token=AAE7CUPHAYJWGT6LFXJD3XTPV2NRNA5CNFSM4HMEUEC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVUSGTQ#issuecomment-493429582>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAE7CUIHQF6IWWINQ6SKBPLPV2NRNANCNFSM4HMEUECQ>
.
|
| this.onFormatChange( record, true ); | ||
| } | ||
|
|
||
| onFormatChange( record ) { |
There was a problem hiding this comment.
@Tug should we rename this to onChange to match the web side?
There was a problem hiding this comment.
We might try now! But I think it's outside the scope of this PR...
On the web onChange accepts a record and is going to apply it to the DOM tree to apply only the required modifications.
Since we don't use the DOM on native, we created our own versions of onChange, one for value change (we kept the name onChange but we accept a native event argument instead) and one for format change (we called onFormatChange) in which we're calling valueToFormat to convert the record back into html to update the value instead of the dom.
|
Thanks for the background info, I think it will be good to add documentation to the methods to explain what do they do. I also suggested the change because normally when we copy the code around from the web we normally see onChange and then need to change it to onFormatChange.
…Sent from my iPhone
On 17 May 2019, at 15:23, Tugdual de Kerviler ***@***.***> wrote:
@Tug commented on this pull request.
In packages/block-editor/src/components/rich-text/index.native.js:
> @@ -255,10 +251,6 @@ export class RichText extends Component {
} ).map( ( name ) => gutenbergFormatNamesToAztec[ name ] ).filter( Boolean );
}
- onFormatChangeForceChild( record ) {
- this.onFormatChange( record, true );
- }
-
onFormatChange( record ) {
We might try now! But I think it's outside the scope of this PR...
On the web onChange accepts a record and is going to apply it to the DOM tree to apply only the required modifications.
Since we don't use a DOM tree on native, we created our own version of onChange, one for value change (we kept the name onChange but we accept a native event argument instead) and one for format change (we called onFormatChange) in which we're calling valueToFormat to convert the record back into html to update the value instead of the dom.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@daniloercoli can you help on the Android side? |
|
@SergioEstevao @Tug - I was able to reproduce #15566 (comment) in Android, iOS, and Web. So I'd say it's a bug on the web side.
EDIT: |
|
Did you try on iOS on pressing backspace on a multi-line para block created on the web? |
I went ahead and try it and it work fines on iOS, because the onDelete event is not sent to the JS side and it's still being processed in Aztec. For the record on iOS the |
|
@daniloercoli update with the latest master can you please give it a look? |
Description
This PR updates the RN list block code to support the following:
How has this been tested?
This can be tested using this PR on the GB-mobile repo.
Screenshots
Types of changes
New feature and bug fix on nested lists.
Checklist: