Skip to content
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] Scroll to new-block indicator #31144

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f055d53
First stab at scrolling to the indicator
hypest Apr 23, 2021
9d6145c
Use insertionPoint selector and scroll in componentDidUpdate
hypest Apr 23, 2021
f6c62d9
Remove unneeded assignment
hypest Apr 23, 2021
a596571
Slice array instead of conditional for limiting accumulation
hypest Apr 23, 2021
76b2594
Keep clientId list so block removals/additions can be correctly calcu…
hypest Apr 23, 2021
371c5e6
Remove unneeded assignment
hypest Apr 23, 2021
9400f23
Remove verbose property passing
hypest Apr 23, 2021
7862e89
Also scroll on iOS
hypest Apr 23, 2021
4da01c9
Use scrollToOffset always on Android to simplify code
hypest Apr 24, 2021
40ccf72
No need for getItemLayout anymore
hypest Apr 24, 2021
1bc5669
Fix innerblocks case to not scroll at all
hypest Apr 24, 2021
68e5118
Set list footer height to a ratio of the list height
hypest Apr 24, 2021
d8a9258
Simplify property passing
hypest Apr 24, 2021
7ab5f03
Rewrite to please the linter
hypest Apr 24, 2021
c396049
Merge branch 'trunk' into rnmobile/1672-scroll-to-new-block-indicator
hypest Apr 24, 2021
c2856b5
Accessibility name included in parents so, grab the innermost
hypest Apr 25, 2021
bfa7db7
Merge branch 'trunk' into rnmobile/1672-scroll-to-new-block-indicator
hypest Jun 4, 2021
718dee1
Remove shouldFlatListPreventAutomaticScroll, always scrolling now
hypest Jun 4, 2021
87456cf
Merge branch 'trunk' into rnmobile/1672-scroll-to-new-block-indicator
hypest Jun 16, 2021
1a3e811
Add relevant entry to the changelog
hypest Jun 16, 2021
00ee6ad
Add some informational inline comments
hypest Jun 16, 2021
6c6f562
Fix lint issue
hypest Jun 16, 2021
e096eb3
Need to re-enable the lint check at some point
hypest Jun 16, 2021
40f86fb
Merge branch 'trunk' into rnmobile/1672-scroll-to-new-block-indicator
hypest Jun 16, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export class BlockListItem extends Component {
marginHorizontal,
blockName,
blockWidth,
onLayout,
...restProps
} = this.props;
const readableContentViewStyle =
Expand All @@ -141,6 +142,7 @@ export class BlockListItem extends Component {
<View
style={ this.getContentStyles( readableContentViewStyle ) }
pointerEvents={ isReadOnly ? 'box-only' : 'auto' }
onLayout={ onLayout }
>
{ shouldShowInsertionPointBefore && (
<BlockInsertionPoint />
Expand Down
96 changes: 83 additions & 13 deletions packages/block-editor/src/components/block-list/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ export class BlockList extends Component {
);
this.scrollViewInnerRef = this.scrollViewInnerRef.bind( this );
this.addBlockToEndOfPost = this.addBlockToEndOfPost.bind( this );
this.shouldFlatListPreventAutomaticScroll = this.shouldFlatListPreventAutomaticScroll.bind(
this
);
this.shouldShowInnerBlockAppender = this.shouldShowInnerBlockAppender.bind(
this
);
Expand All @@ -80,6 +77,10 @@ export class BlockList extends Component {

this.onLayout = this.onLayout.bind( this );

this.listHeight = 0;
this.offsetOfIndex = this.offsetOfIndex.bind( this );
this.itemHeights = [];

this.state = {
blockWidth: this.props.blockWidth || 0,
};
Expand All @@ -102,10 +103,6 @@ export class BlockList extends Component {
this.scrollViewRef = ref;
}

shouldFlatListPreventAutomaticScroll() {
return this.props.isBlockInsertionPointVisible;
}

shouldShowInnerBlockAppender() {
const { blockClientIds, renderAppender } = this.props;
return renderAppender && blockClientIds.length > 0;
Expand Down Expand Up @@ -155,6 +152,9 @@ export class BlockList extends Component {
const { blockWidth } = this.state;
const { isRootList, maxWidth } = this.props;

// update the known list height. Using it to compute how much empty space to reserve in the footer
this.listHeight = layout.height;

const layoutWidth = Math.floor( layout.width );
if ( isRootList && blockWidth !== layoutWidth ) {
this.setState( {
Expand All @@ -165,6 +165,59 @@ export class BlockList extends Component {
}
}

/* eslint-disable jsdoc/require-param */
/**
* Computes the offset of a block in pixels, by adding up all the block heights before it.
*/
/* eslint-enable jsdoc/require-param */
offsetOfIndex( heights, blockClientIds, index ) {
const ITEM_HEIGHT = 30; // just a kinda arbitrary default height, just to have it non zero.
const offset = blockClientIds
.slice( 0, index + 1 ) // only add up to the index we want (adding 1 to include the indexed item itself too)
.reduce(
// accumulate the heights of the items
( acc, id ) =>
heights[ id ] && heights[ id ] > 0
? acc + heights[ id ]
: acc + ITEM_HEIGHT,
0
);
return offset;
}

componentDidUpdate( prevProps ) {
const {
isBlockInsertionPointVisible,
insertionPoint,
blockClientIds,
} = this.props;

// if we're now showing the new-block indicator, trigger a scroll to it
if (
isBlockInsertionPointVisible !==
prevProps.isBlockInsertionPointVisible
) {
const insertionPointInRootList =
insertionPoint.rootClientId === undefined;
if ( insertionPointInRootList && isBlockInsertionPointVisible ) {
const jumpToIndex = insertionPoint.index - 1; // scrolling goes to the bottom of the item so, let's scroll to one above
const offset =
jumpToIndex < 0
? 0
: this.offsetOfIndex(
this.itemHeights,
blockClientIds,
jumpToIndex
);
if ( Platform.OS === 'android' ) {
this.scrollViewRef.scrollToOffset( { offset } );
} else {
this.scrollViewRef.scrollTo( { y: offset } );
}
}
}
}

render() {
const { isRootList } = this.props;
// Use of Context to propagate the main scroll ref to its children e.g InnerBlocks
Expand Down Expand Up @@ -276,9 +329,6 @@ export class BlockList extends Component {
data={ blockClientIds }
keyExtractor={ identity }
renderItem={ this.renderItem }
shouldPreventAutomaticScroll={
this.shouldFlatListPreventAutomaticScroll
}
title={ title }
ListHeaderComponent={ header }
ListEmptyComponent={ ! isReadOnly && this.renderEmptyList }
Expand Down Expand Up @@ -332,6 +382,11 @@ export class BlockList extends Component {
this.shouldShowInnerBlockAppender
}
blockWidth={ blockWidth }
onLayout={
( object ) =>
( this.itemHeights[ clientId ] =
object.nativeEvent.layout.height ) // Capture the block height. We'll use the list of heights to compute offsets.
}
/>
);
}
Expand All @@ -345,6 +400,11 @@ export class BlockList extends Component {
} = this.props;

if ( ! isReadOnly && withFooter ) {
const footerHeight = Math.max(
( this.listHeight * 3 ) / 4, // set the footer to 3 quarters of the list height to give room for the inserter *plus* the insertion point
styles.blockListFooter.minHeight
);

return (
<>
<TouchableWithoutFeedback
Expand All @@ -354,7 +414,12 @@ export class BlockList extends Component {
this.addBlockToEndOfPost( paragraphBlock );
} }
>
<View style={ styles.blockListFooter } />
<View
style={ [
styles.blockListFooter,
Copy link
Member

Choose a reason for hiding this comment

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

It would be ideal to only apply this offset when the inserter is opened and animate it to avoid a jump, but I would imagine that might be difficult to accomplish. I wanted to provide the thought nonetheless. Maybe an opportunity for future enhancement.

{ height: footerHeight },
] }
/>
</TouchableWithoutFeedback>
</>
);
Expand All @@ -372,6 +437,7 @@ export default compose( [
getBlockCount,
getBlockOrder,
getSelectedBlockClientId,
getBlockInsertionPoint,
isBlockInsertionPointVisible,
getSettings,
} = select( blockEditorStore );
Expand All @@ -394,11 +460,15 @@ export default compose( [

const isFloatingToolbarVisible =
!! selectedBlockClientId && hasRootInnerBlocks;

const insertionPoint = getBlockInsertionPoint();

return {
blockClientIds,
blockCount,
isBlockInsertionPointVisible:
Platform.OS === 'ios' && isBlockInsertionPointVisible(),
insertionPoint,
isBlockInsertionPointVisible: isBlockInsertionPointVisible(),
selectedBlockClientId,
isReadOnly,
isRootList: rootClientId === undefined,
isFloatingToolbarVisible,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
}

.blockListFooter {
height: 80px;
min-height: 80px;
}

.defaultBlock {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import KeyboardAvoidingView from '../keyboard-avoiding-view';

export const KeyboardAwareFlatList = ( props ) => (
<KeyboardAvoidingView style={ { flex: 1 } }>
<FlatList { ...props } />
<FlatList ref={ props.innerRef } { ...props } />
</KeyboardAvoidingView>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const List = memo( FlatList, isEqual );

export const KeyboardAwareFlatList = ( {
extraScrollHeight,
shouldPreventAutomaticScroll,
innerRef,
autoScroll,
scrollViewStyle,
Expand All @@ -41,8 +40,7 @@ export const KeyboardAwareFlatList = ( {
setTimeout( () => {
if (
! this.keyboardWillShowIndicator &&
this.latestContentOffsetY !== undefined &&
! shouldPreventAutomaticScroll()
this.latestContentOffsetY !== undefined
) {
// Reset the content position if keyboard is still closed
if ( this.scrollViewRef ) {
Expand Down
1 change: 1 addition & 0 deletions packages/react-native-editor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ For each user feature we should also add a importance categorization label to i
- [*] Gallery block - Fix gallery images caption text formatting [#32351]
- [*] Image block: "Set as featured" button within image block settings. (Android only) [#31705]
- [***] Audio block now available on WP.com sites on the free plan. [#31966]
- [**] Scrolling to the new-block indicator when adding a block. [#31144]

## 1.54.0
- [***] Slash inserter [#29772]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ class EditorPage {
}

async getFirstBlockVisible() {
const firstBlockLocator = `//*[contains(@${ this.accessibilityIdXPathAttrib }, " Block. Row ")]`;
const firstBlockLocator = `//*[contains(@${ this.accessibilityIdXPathAttrib }, " Block. Row 1")]`; // notice the `1`, looking only for the first block but the "Block. Row 1" text is there in many of the parent components
const elements = await this.driver.elementsByXPath( firstBlockLocator );
return elements[ 0 ];
return elements[ elements.length - 1 ]; // return the last found as it will be the innermost
}

async getLastBlockVisible() {
Expand Down