Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RNMobile] Inner Block Navigate down through hierarchy #17547
[RNMobile] Inner Block Navigate down through hierarchy #17547
Changes from 71 commits
6da1bf4
6387115
e010328
09359f6
a424c63
2e7204b
5dce162
76340c0
3295957
a28b17d
f00ad33
5326d44
c70697c
eb7bed0
2fdedb7
d602fff
497dcbc
ca22502
dd06cba
ec97815
3bf893b
8b02aae
4c1e1b9
e5976c0
b54fa56
a6dcf70
63b163f
d7434b0
ed9a435
aafa452
dffabf9
4576c6a
90ae894
d8d4e1d
677aade
168c3a2
d8b05e2
e8464a0
7a500e1
132fa7a
f94c828
845f920
35d8c8b
7eba06d
48a62f4
65b8400
bd2a9fc
c072555
f06f6af
f2be82d
e47908d
e8ae8b2
84d8dad
014e3a0
fdb2fae
3b94490
941baef
df0f5a4
8ab7ebe
3ebc50c
a035030
cd5b608
bd5a1f8
3219ad0
e3b540a
f47e6dc
ca7e08f
8de07ef
93ae250
662008e
6414b89
524e0ca
7cf3950
5a456c9
061257a
7fb084a
583650b
4574daf
22d5904
75482c5
0814bb8
7cb1ddb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT about create const with
getBlockParents( selectedBlockClientId )
and use it instead of calling it 2 times?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reading this comment, it did look a bit related to the
firstToSelect
search, so I think we could have the equivalent (please double check my logic here 😉):isDescendantSelected
: if the selection is one of the descendants, then this must be a parent of the selection, which also means it will be the lowest common ancestor. SoisDescendantSelected = commonAncestor === clientId
isDescendantOfParentSelected
: similar to the previous one, if the parent of this block is an ancestor of the selection, but the selection is not one of this block's descendants, then the lowest common ancestor if this block's parent:isDescendantOfParentSelected = commonAncestor === parentId
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@koke I temporary go with @dratwas suggest.
However I agree with this one
isDescendantSelected = commonAncestor === clientId
but not sure about this
isDescendantOfParentSelected = commonAncestor === parentId
Shouldn't we check the common ancestor with parent of selected block ?
Then it will look like this:
const commonAncestorParent = getLowestCommonAncestorWithSelectedBlock( parentId );
const isDescendantOfParentSelected = commonAncestorParent === parentId;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I think that would change the behavior a bit. The
commonAncestor == parentId
would not include the case whereisDescendantSelected == true
. It would be more accurate to sayisDescendantOfSiblingSelected
, which might not be what you had in mind when doing the calculations. I think we can combine both:But at this stage, this feels more like nitpicking, so I'm happy with either approach, we shouldn't block on this one to merge.