Skip to content

Inner blocks: Reduce tree depth to improve performance. #50447

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.

Sign up for GitHub

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

Merged
dcalhoun merged 19 commits into from
May 12, 2023
Merged

Inner blocks: Reduce tree depth to improve performance. #50447

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
aaaf421
test: Await overlooked asynchronous task
dcalhoun May 8, 2023
517d99f
feat: Combine BlockList and BlockListCompact
dcalhoun May 8, 2023
1dc9a1b
wip: Gallery, Columns working; Buttons broken
dcalhoun May 8, 2023
035d8cc
test: Fix nested lists tests
dcalhoun May 9, 2023
5b59fd4
wip: Fix Blocks button layout
dcalhoun May 9, 2023
31755d9
refactor: Remove unused BlockList title prop
dcalhoun May 9, 2023
a9c20ef
refactor: Separate inner block list styles
dcalhoun May 9, 2023
d618a5c
refactor: Remove unused virtualized listKey
dcalhoun May 9, 2023
8ac334c
refactor: Remove outdated inline comment
dcalhoun May 9, 2023
33ca7c8
wip: Explore nest Columns fixes
dcalhoun May 10, 2023
4647601
Mobile - BlockList - Fix layout issues for stacked horizontally blocks
May 10, 2023
b49b9b8
fix: Revert Buttons alignment workaround
dcalhoun May 10, 2023
2d79f7a
fix: Column width sums less than 100% correct align
dcalhoun May 10, 2023
621b244
refactor: Inner blocks use clientId key
dcalhoun May 10, 2023
445a5b1
refactor: Remove unnecessary key
dcalhoun May 10, 2023
7618279
fix: Prevent block list footer from stretching to parent height
dcalhoun May 10, 2023
b46922b
Mobile - List block - Remove usage of useCompactList and disables the…
May 11, 2023
73996cb
test: Update Android e2e Buttons block Xpath selector
dcalhoun May 11, 2023
f645983
test: Delay query for block actions menu
dcalhoun May 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat: Combine BlockList and BlockListCompact 
Reduce code duplication between the separate components, e.g. item
render, footer, end-of-list block appender button, empty list
placeholder.
  • Loading branch information
@dcalhoun
dcalhoun committed May 10, 2023
commit 517d99f9274226afe24a290908fd62b7676d8c43
63 changes: 0 additions & 63 deletions packages/block-editor/src/components/block-list/block-list-compact.native.js
View file
Open in desktop

This file was deleted.

278 changes: 149 additions & 129 deletions packages/block-editor/src/components/block-list/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import BlockListItem from './block-list-item';
import BlockListItemCell from './block-list-item-cell';
import {
BlockListProvider,
BlockListConsumer,
DEFAULT_BLOCK_LIST_CONTEXT,
} from './block-list-context';
import { BlockDraggableWrapper } from '../block-draggable';
Expand Down Expand Up @@ -190,114 +189,6 @@ export default function BlockList( {
}
};

const renderList = ( extraProps = {} ) => {
const { parentScrollRef, onScroll } = extraProps;

const { blockToolbar, headerToolbar, floatingToolbar } = styles;

const containerStyle = {
flex: isRootList ? 1 : 0,
// We set negative margin in the parent to remove the edge spacing between parent block and child block in ineer blocks.
marginVertical: isRootList ? 0 : -marginVertical,
marginHorizontal: isRootList ? 0 : -marginHorizontal,
};

const isContentStretch = contentResizeMode === 'stretch';
const isMultiBlocks = blockClientIds.length > 1;
const { isWider } = alignmentHelpers;
const extraScrollHeight =
headerToolbar.height +
blockToolbar.height +
( isFloatingToolbarVisible ? floatingToolbar.height : 0 );

const scrollViewStyle = [
{ flex: isRootList ? 1 : 0 },
! isRootList && styles.overflowVisible,
];

return (
<View
style={ containerStyle }
onAccessibilityEscape={ clearSelectedBlock }
onLayout={ onLayout }
testID="block-list-wrapper"
>
<KeyboardAwareFlatList
{ ...( Platform.OS === 'android'
? { removeClippedSubviews: false }
: {} ) } // Disable clipping on Android to fix focus losing. See https://github.com/wordpress-mobile/gutenberg-mobile/pull/741#issuecomment-472746541
accessibilityLabel="block-list"
innerRef={ ( ref ) => {
scrollViewRef.current = parentScrollRef || ref;
} }
extraScrollHeight={ extraScrollHeight }
keyboardShouldPersistTaps="always"
scrollViewStyle={ scrollViewStyle }
extraData={ getExtraData() }
scrollEnabled={ isRootList }
contentContainerStyle={ [
horizontal && styles.horizontalContentContainer,
isWider( blockWidth, 'medium' ) &&
( isContentStretch && isMultiBlocks
? styles.horizontalContentContainerStretch
: styles.horizontalContentContainerCenter ),
] }
style={ getStyles(
isRootList,
isStackedHorizontally,
horizontalAlignment
) }
data={ blockClientIds }
keyExtractor={ identity }
listKey={
rootClientId ? `list-${ rootClientId }` : 'list-root'
}
Comment on lines -252 to -254
Copy link
Member Author

@dcalhoun dcalhoun May 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer necessary now that only the root list is virtualized.

renderItem={ renderItem }
CellRendererComponent={ BlockListItemCell }
shouldPreventAutomaticScroll={
shouldFlatListPreventAutomaticScroll
}
title={ title }
ListHeaderComponent={ header }
ListEmptyComponent={
! isReadOnly && (
<EmptyList
orientation={ orientation }
rootClientId={ rootClientId }
renderAppender={ renderAppender }
renderFooterAppender={ renderFooterAppender }
/>
)
}
ListFooterComponent={
<Footer
addBlockToEndOfPost={ addBlockToEndOfPost }
isReadOnly={ isReadOnly }
renderFooterAppender={ renderFooterAppender }
withFooter={ withFooter }
/>
}
onScroll={ onScroll }
/>
{ shouldShowInnerBlockAppender() && (
<View
style={ {
marginHorizontal:
marginHorizontal -
styles.innerAppender.marginLeft,
} }
>
<BlockListAppender
rootClientId={ rootClientId }
renderAppender={ renderAppender }
showSeparator
/>
</View>
) }
</View>
);
};

const renderItem = ( { item: clientId, index } ) => {
// Extracting the grid item properties here to avoid
// re-renders in the blockListItem component.
Expand All @@ -311,6 +202,7 @@ export default function BlockList( {
<BlockListItem
index={ index }
isStackedHorizontally={ isStackedHorizontally }
key={ clientId }
rootClientId={ rootClientId }
clientId={ clientId }
parentWidth={ parentWidth }
Expand All @@ -328,29 +220,157 @@ export default function BlockList( {
);
};

const { blockToolbar, headerToolbar, floatingToolbar } = styles;

const containerStyle = {
flex: isRootList ? 1 : 0,
// We set negative margin in the parent to remove the edge spacing between parent block and child block in ineer blocks.
marginVertical: isRootList ? 0 : -marginVertical,
marginHorizontal: isRootList ? 0 : -marginHorizontal,
};

const isContentStretch = contentResizeMode === 'stretch';
const isMultiBlocks = blockClientIds.length > 1;
const { isWider } = alignmentHelpers;
const extraScrollHeight =
headerToolbar.height +
blockToolbar.height +
( isFloatingToolbarVisible ? floatingToolbar.height : 0 );

const scrollViewStyle = [
{ flex: isRootList ? 1 : 0 },
! isRootList && styles.overflowVisible,
];

// Use of Context to propagate the main scroll ref to its children e.g InnerBlocks.
const blockList = isRootList ? (
<BlockListProvider
value={ {
...DEFAULT_BLOCK_LIST_CONTEXT,
scrollRef: scrollViewRef.current,
} }
return (
<View
style={ containerStyle }
onAccessibilityEscape={ clearSelectedBlock }
onLayout={ onLayout }
testID="block-list-wrapper"
>
<BlockDraggableWrapper isRTL={ isRTL }>
{ ( { onScroll } ) => renderList( { onScroll } ) }
</BlockDraggableWrapper>
</BlockListProvider>
) : (
<BlockListConsumer>
{ ( { scrollRef } ) =>
renderList( {
parentScrollRef: scrollRef,
} )
}
</BlockListConsumer>
{ isRootList ? (
<BlockListProvider
value={ {
...DEFAULT_BLOCK_LIST_CONTEXT,
scrollRef: scrollViewRef.current,
} }
>
<BlockDraggableWrapper isRTL={ isRTL }>
{ ( { onScroll } ) => (
<KeyboardAwareFlatList
{ ...( Platform.OS === 'android'
? { removeClippedSubviews: false }
: {} ) } // Disable clipping on Android to fix focus losing. See https://github.com/wordpress-mobile/gutenberg-mobile/pull/741#issuecomment-472746541
accessibilityLabel="block-list"
innerRef={ ( ref ) => {
scrollViewRef.current = ref;
} }
extraScrollHeight={ extraScrollHeight }
keyboardShouldPersistTaps="always"
scrollViewStyle={ scrollViewStyle }
extraData={ getExtraData() }
scrollEnabled={ isRootList }
contentContainerStyle={ [
horizontal &&
styles.horizontalContentContainer,
isWider( blockWidth, 'medium' ) &&
( isContentStretch && isMultiBlocks
? styles.horizontalContentContainerStretch
: styles.horizontalContentContainerCenter ),
] }
style={ getStyles(
isRootList,
isStackedHorizontally,
horizontalAlignment
) }
data={ blockClientIds }
keyExtractor={ identity }
listKey={
rootClientId
? `list-${ rootClientId }`
: 'list-root'
}
renderItem={ renderItem }
CellRendererComponent={ BlockListItemCell }
shouldPreventAutomaticScroll={
shouldFlatListPreventAutomaticScroll
}
title={ title }
ListHeaderComponent={ header }
ListEmptyComponent={
! isReadOnly && (
<EmptyList
orientation={ orientation }
rootClientId={ rootClientId }
renderAppender={ renderAppender }
renderFooterAppender={
renderFooterAppender
}
/>
)
}
ListFooterComponent={
<Footer
addBlockToEndOfPost={
addBlockToEndOfPost
}
isReadOnly={ isReadOnly }
renderFooterAppender={
renderFooterAppender
}
withFooter={ withFooter }
/>
}
onScroll={ onScroll }
/>
) }
</BlockDraggableWrapper>
</BlockListProvider>
) : (
<>
{ blockClientIds.length > 0 ? (
<>
{ blockClientIds.map( ( currentClientId, index ) =>
renderItem( {
item: currentClientId,
index,
} )
) }
<Footer
addBlockToEndOfPost={ addBlockToEndOfPost }
isReadOnly={ isReadOnly }
renderFooterAppender={ renderFooterAppender }
withFooter={ withFooter }
/>
</>
) : (
<EmptyList
orientation={ orientation }
rootClientId={ rootClientId }
renderAppender={ renderAppender }
renderFooterAppender={ renderFooterAppender }
/>
) }
</>
) }
{ shouldShowInnerBlockAppender() && (
<View
style={ {
marginHorizontal:
marginHorizontal - styles.innerAppender.marginLeft,
} }
>
<BlockListAppender
rootClientId={ rootClientId }
renderAppender={ renderAppender }
showSeparator
/>
</View>
) }
</View>
);

return blockList;
}

function Footer( {
Expand Down
Loading