Implement list block in React Native #14636
Conversation
SergioEstevao
added
the
Mobile App - i.e. Android or iOS
SergioEstevao
requested review from
gziolo,
noisysocks,
talldan and
youknowriad
as code owners
| return { | ||
| clientId: context.clientId, | ||
| isSelected: context.isSelected, | ||
| onFocus: context.onFocus, |
There was a problem hiding this comment.
In the web implementation context.setFocusedElement is used instead to mark which RichText component is active:
There was a problem hiding this comment.
I saw that but it looks we don't use the same logic on the native part on our block manager.
There was a problem hiding this comment.
how is going your solution work with multiple RichText controls included in a single block?
|
Good question, but we are still not there. I agree that in the future we
will need to change our implementation to do the same has web. But at the
moment this is what we have available.
…On Wed, 27 Mar 2019 at 10:48, Grzegorz (Greg) Ziółkowski < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In packages/block-editor/src/components/rich-text/index.native.js
<#14636 (comment)>:
> + clientId: context.clientId,
+ };
+ }
+ // When explicitly set as selected, use the value stored in the context instead.
+ if ( ownProps.isSelected === true ) {
+ return {
+ isSelected: context.isSelected,
+ clientId: context.clientId,
+ };
+ }
+
+ // Ensures that only one RichText component can be focused.
+ return {
+ clientId: context.clientId,
+ isSelected: context.isSelected,
+ onFocus: context.onFocus,
how is going your solution work with multiple RichText controls included
in a single block?
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#14636 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAnxUSRovo2txjSZxS_9iH--TSGzS2khks5va0x4gaJpZM4cLd7C>
.
|
|
Pushed 08318ca which includes a fix for the toggling of list type in nested lists. Known issues:
|
|
@hypest Could you perhaps also update the web version? |
| * @return {boolean} True if the root list or nothing is selected, false if an | ||
| * inner list is selected. | ||
| */ | ||
| function isListRootSelected( value ) { |
There was a problem hiding this comment.
Why not replace getStartListFormat with both isListRootSelected and isActiveListType? This seems weird to me.
There was a problem hiding this comment.
👋 @ellatrix , not sure I follow. Do you mean to completely remove getStartListFormat and implement its pieces inside isListRootSelected and isActiveListType?
There was a problem hiding this comment.
Yes, export isListRootSelected and isActiveListType from rich-text as unstable.
| @@ -6,7 +6,7 @@ import { Toolbar } from '@wordpress/components'; | |||
| import { __ } from '@wordpress/i18n'; | |||
| import { | |||
| changeListType, | |||
| getStartListFormat, | |||
| getLineListFormat, | |||
There was a problem hiding this comment.
I meant to rename it as getLineNestingLevel, back when it was only about the nesting. I think it's better to export two functions instead of overloading one for multiple purposes. 66e971d#r272474533
There was a problem hiding this comment.
Aha, I see. I will work on that.
|
@hypest Ok. I thought it's useful to know that the functions you introduce work in both scenarios. |
Yes, that's perfect information to know @ellatrix , thanks for assessing this! |
Description
This PR implements the list block by only changing components and not touching the code of the block ( no need to create a native version).
This achieved by improving the implementation of the block-edit context in RNMobile in order to send the isSelected and onFocus props to the RichText components.
For the moment it doesn't implement the indent and outdent buttons, but those should be relatively after a better understanding of the format and selection code behind it.At the moment the block allows the breaking of the list by tapping two enters in a row on an empty list element. This can be solved by changing Aztec to send the event up instead of breaking the list by itself.How has this been tested?
This can be tested using this PR on gb-mobile: wordpress-mobile/gutenberg-mobile#704