[Mobile] - Drag & drop blocks - Fetch and share blocks layout size and position coordinates #39089
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
February 22, 2022 13:15
… and add support to store the blocks layouts data and coordinates.
…ata and use updateBlocksLayouts to store it. It is needed to use CellRendererComponent to be able to get the right position coordinates
geriux
added
[Status] In Progress
|
Size Change: 0 B Total Size: 1.16 MB ℹ️ View Unchanged
|
geriux
added
the
Mobile App - Automation
fluiddot
reviewed
Mar 1, 2022
packages/block-editor/src/components/block-list/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/block-list-item-cell.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/block-list-context.native.js
Outdated
Show resolved
Hide resolved
fluiddot
reviewed
Mar 1, 2022
There was a problem hiding this comment.
I tested the blocksLayouts data flow and works as expected 🎊 . Besides, I validated that using the hook useBlockListContext in a testing component, under the root block list, the data can be retrieved. Once we review the comments I posted in the first review, the PR would be ready to merge from my side.
One thing we might need in the future is the absolute position of the elements, as currently, the x and y values received from onLayout event is the relative position. Let's see when we implement the rest of the drag & drop functionality if we really need it.
On the other side of things, probably it won't impact too much on performance, as it's won't cause re-renders, but I'd expect that having onLayout events on every block would introduce a bit of overhead when selecting a block. For example, if you have 100 blocks and you select the first one, it will trigger the onLayout event on the 100 blocks because the selected block gets bigger and pushes the rest of the blocks. Nevertheless, I don't think we have an alternative as we need these calculations 😅. For now, let's just keep in mind that performance should be tested during the testing phase of the drag&drop feature.
added 5 commits
March 2, 2022 16:46
Thanks for the review and testing! I applied the suggestions and its ready for another review =)
Yeah we could remove it if we don't need it at all.
Definitely, we need to check any preformance issues that might occur with these new changes, as you said we do need that data for the calculations so let's monitor during the developing of the feature. |
fluiddot
reviewed
Mar 3, 2022
There was a problem hiding this comment.
Definitely, we need to check any preformance issues that might occur with these new changes, as you said we do need that data for the calculations so let's monitor during the developing of the feature.
Regarding performance concerns, since currently this PR will be merged into trunk, I'm wondering if we should merge it to a feature branch instead, in case we're unsure how it will impact performance, wdyt?
On the other hand, I've just added some comments, most of them are nitpicks so feel free to ignore them if you wish.
packages/block-editor/src/components/block-list/block-list-context.native.js
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/block-list-context.native.js
Show resolved
Hide resolved
added 4 commits
March 15, 2022 15:46
I created a feature branch: @fluiddot 👋 This is ready for another review whenever you have some time. Thanks! |
fluiddot
approved these changes
Mar 16, 2022
packages/block-editor/src/components/block-list/test/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/test/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/test/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/test/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/test/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/test/block-list-context.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/test/block-list-context.native.js
Outdated
Show resolved
Hide resolved
geriux
requested review from
getdave,
noisysocks,
ellatrix,
mkaz,
ryanwelcher and
juanmaguitar
as code owners
geriux
force-pushed
the
rnmobile/feature/drag-and-drop-fetch-share-blocks-layouts
branch
from
eb6e3c9 to
d8f7cf8
Compare
geriux
removed request for
juanmaguitar,
TimothyBJacobs,
spacedmonkey,
antonis,
ajitbohra,
ntwb,
nerrad,
draganescu,
talldan,
tellthemachines,
adamziel,
kevin940726,
getdave,
noisysocks,
ellatrix,
mkaz and
ryanwelcher
…rag-and-drop-fetch-share-blocks-layouts
geriux
deleted the
rnmobile/feature/drag-and-drop-fetch-share-blocks-layouts
branch
fluiddot
added a commit
that referenced
this pull request
May 11, 2022
* [Mobile] - Drag & drop blocks - Fetch and share blocks layout size and position coordinates (#39089) * Mobile - Block list - Extract block list context into a separate file and add support to store the blocks layouts data and coordinates. * Mobile - Block list - Adds block list item cell to get the onLayout data and use updateBlocksLayouts to store it. It is needed to use CellRendererComponent to be able to get the right position coordinates * Mobile - Block list - Store block layouts data for inner blocks in a deep level * Mobile - BlockList ItemCell - Destructuring props * Mobile - BlockListContext - Rename findByRootId to findBlockLayoutByClientId * Mobile - BlockListContext - Rename deleteByClientId to deleteBlockLayoutByClientId * Mobile - BlockListContext - Store default context and use it for initialization * Mobile - BlockListContext - Add param's docs * Mobile - Block list context - Export findBlockLayoutByClientId * Mobile - Block list context - Update comments * Mobile - Block list context - Unit tests * Mobile - Block list context - update unit tests * [Mobile] - Draggable component (#39551) * Mobile - Add Draggable component * Mobile - Draggable - Fix composition of gestures, enable Pan gesture once LongPress is recognized * Mobile - Draggable component - Track if the gesture Pan started, if it didn't and long pressure was activated it should call onDragEnd * Mobile - Draggable component - Add props documentation * Mobile - Draggable component - Update documentation * Mobile - Draggable component - Refactor that includes: - Usage of only one onEnd callback for both gestures. - Removes the hasPanStarted value since the onEnd callback is unified - Adds shouldCancelWhenOutside for the Pan gesture. - Removes the simultaneousWithExternalGesture since the Pan gesture is manually activated and composed with the LongPress gesture. - Add isIOS check within onTouchesMove for the state.fail() call. * Mobile - Draggable component - Use Platform from @wordpress/element * [RNMobile] `BlockDraggable` component (#39617) * Use animated scroll handler in KeyboardAwareFlatList * Add hook for scrolling the block list while dragging * Improve scroll animation in useScrollWhenDragging * Add draggable chip component * Add block draggable component * Remove icon prop from draggable chip component * Add draggable placeholder * Fix draggable chip location * Wrap BlockListItemCell with BlockDraggable * Fix block draggable placeholder style * Animate scale property instead of opacity of draggable chip * Fix draggable placeholder container height calculation * Fix BlockDraggable height animation * Move draggable to BlockDraggableWrapper * Disable isDragging when long-press gesture ends * Fix onLayout calculation in block list item cell * Add findBlockLayoutByPosition helper * Set up dragging block by position * Remove animate scroll velocity * Remove useScrollWhenDragging hook This hook will be introduced in a separate PR * Remove react-native-reanimated mock * Rename CHIP_OFFSET_TO_TOUCH_POSITION constant * Remove unused shared values of chip component * Stop dragging when no block is found * Fix drag position calculation * Update html text input styles * Unify container component within html text input * Use only a single client id in block draggable * Add documentation to block draggable components * Add documentation to block draggable chip component * Add documentation to findBlockLayoutByPosition * Update scrollOffsetTarget calculation * Fix typos in block draggable * Add draggable wrapper container style * Add dark mode styles for draggable chip * Add dark mode styles for block draggable * Get container height from blocks layout data * Replace inline callback functions with useCallback hook * Update collapse/expand animation when dragging a block * Force draggable chip to be displayed upfront * Remove refs from dependencies arrays References can be omitted from the dependencies arrays since React guarantees that they are inmutable. * [RNMobile] Add `useScrollWhenDragging` hook (#39705) * Introduce useScrollWhenDragging hook * Cancel animation timer on stop scrolling * Add documentation to useScrollWhenDragging hook * Replace Dimensions with useWindowDimensions hook * [RNMobile] Prevent draggable gesture when any text input is focused (#39890) * [Mobile] Adds useBlockDropZone and DroppingInsertionPoint (#39891) * Adds useBlockDropZone and DroppingInsertionPoint * Fix dropping insertion point position when scrolling * Mobile - DroppingInsertionPoint - Fixes: - Avoid showing the indicator when no blocks are being dragged. - Allow showing the dropping indicator at the end of the content. - Prevent checks within useState for isBlockBeingDragged, if no blocks are being dragged. - Prevent looking for a block layout if the clientId is undefined. * Mobile - DroppingInsertionPoint - Add documentation * Mobile - useBlockDropZone - Add documentation * Mobile - useBlockDropZone - Add missing dependencies * Mobile - Block list context - Updates getBlockLayoutsOrderedByYCoord to make it compatible with hermes, currently it doesn't support using the native .sort. It also adds documentation of the function. * Mobile - Updates: useBlockDropZone: - Avoid using showInsertionPoint to avoid re-renders and bad performance. - Passes the current target index as a shared value to pass it to DroppingInsertionPoint. DroppingInsertionPoint: - Detects when targetBlockIndex changes from the dragging animation to update the indicator's position. - Fixes an issue where the last element couldn't be reached. * Mobile - Block list - Revert changes for the insertion point checks * Mobile - Updates documentation: - BlockListContext: Some typos and clarifications. - DroppingInsertionPoint: Update component description. - useBlockDropZone: Update comments. * Mobile - DroppingInsertionPoint - Remove usage of useCallback * Mobile - DroppingInsertionPoint - Remove static styles from useAnimatedStyles and merge both in a separate constant. * Mobile - DroppingInsertionPoint - Move component to the BlockDraggable folder, which is where it's rendered from. * Mobile - DroppingInsertionPoint - Remove usage of hasStartedDraggingOver in favor of isDragging. * Mobile - DroppingInsertionPoint - Remove unneeded braces. Co-authored-by: Carlos Garcia <[email protected]> * [RNMobile] Add `useOnBlockDrop` hook to update blocks order when dropping a block (#39884) * Add useOnBlockDrop hook * Add currentClientId ref to block draggable * Add onBlockDrop to block drop zone hook * Trigger onBlockDrop event when dragging finishes * Fix content overflow when dragging a selected block * [RNMobile] Update drag & drop animations (#40104) * Update drag&drop block animation * Keep block layout data instead of clientId in block draggable * Automatically select the dropped block * Delay opacity animation when dropping a block * Check current client Id existence on stop dragging * [RNMobile] Disable text editing when long-pressing a block in favor of drag & drop gesture (#40101) * Reduce default min duration for long-press gesture * Consume long click event on Android if Aztec text input is not focused * Ensure that drag events are not executed when text input focused * Replace onLongPress with long-press gesture handler in Button component * Use LongPressGestureHandler in button component * Update block-mover snapshots * [RNMobile] Refactor draggable logic and introduce `DraggableTrigger` component (#40406) * Present block mover picker only after state updates * Refactor draggable component * Use DraggableTrigger in BlockDraggable * Move BlockDraggable render to BlockListBlock component * Fix long-press gesture when editing a text on iOS * Memoize draggable provider value to prevent re-renders * Fix dragging not being disabled after scrolling * Reduce calls to event handlers of pan and long-press gestures * Prevent onDragEnd event to be called upon mounting * Add DEFAULT_IOS_LONG_PRESS_MIN_DURATION constant * [RNMobile] Update animation of drag & drop chip component (#40409) * Use layout animations when rendering the chip component Additionally, the block icon is now provided from the BlockDraggable component. * Fix chip layout calculation * Set block icon as state value * Update enter/exit animation duration of chip component * Mobile - DroppingInsertionPoint - Update indicator position for selected blocks (#40510) * Mobile - DroppingInsertionPoint - Update the indicator's position for the current selected block for both top and bottom positions depending on the current position when dragging. * Replace parseInt to Math.floor * [RNMobile] Add haptic feedback to drag & drop feature (#40423) * Add generate haptic feedback function into RN bridge * Add haptic feedback to drag & drop * Reduce haptic feedback intensity on iOS * [RNMobile] Fix issues related to editing text and dragging gesture (#40480) * Add input state functionality to Aztec * Control drag enabling with Aztec input state * Force disabling text editing when dragging * Add documentation to AztecInputState * Update changelog * Add tests for Aztec input state * Update focus change listener logic * Update listen to focus change event test * Fix react-native-aztec module mock * Fix wrong call to removeFocusChangeListener * Fix updating currentFocusedElement value * Check if an inner block is selected when enabling dragging * Wrap draggable long-press handler with useCallback * Add documentation to notifyListeners function * Mobile - Draggable - Disable multiple touches (#40519) * Mobile - Draggable - Disable multiple touches by getting the first touch during onTouchesMove, since using the maxPointers config works unexpectedly on Android. * Mobile - Draggable - Order touch events by ID and use the first element * Pass isPanActive to the LongPress onEnd callback to not update the isDragging flag when the panning event is active * Mobile - Draggable - Store the first touch ID instead of picking the first event within the allTouches event array. * Mobile - DroppingInsertionPoint - Hide indicator when it overflows outside the content (#40658) * Mobile - Provider - Adds SafeAreaProvider * Mobile - DroppingInsertionPoint - Hide indicator if the current position overflows over the header or footer * Fix tests, adds react-native-safe-area-context mock * Mobile - DroppingInsertionPoint - Use the height value from the useSafeAreaFrame instead * Mobile - DroppingInsertionPoint - Update mocks * Mobile - Disable long press event in media blocks (#40651) * Mobile - Disable long press event in media blocks * Mobile - Media & Text - Remove extra param * Mobile - Media & Text - Show replace media button for both Image and video * Mobile - DraggingInsertionPoint - Prevent crash when accessing a null element (#40689) * Mobile - DraggingInsertionPoint - Fix crash when there's only one block in the editor, the previousElement is null. * Mobile - DroppingInsertionPoint - Avoid having NaN values * Mobile - Draggable - Add onTouchesCancelled to handle manually ending the drag & drop event in cases like sending the app to background or opening the notifications center on iOS (#40729) * [RNMobile] Fix Android crash when closing the editor while dragging (#40810) * Remove Reanimated transitive dependency on Android * Test Android crash fix by changing Reanimated version * Bump react-native-reanimated dependency version * Bump react-native-reanimated dependency on Android * Update package-lock.json file * Bump react-native-reanimated dependency version * Revert "Test Android crash fix by changing Reanimated version" This reverts commit d01cdae. * Mobile - AztecView - Trigger notifyInputChange on focus/blur (#40788) * Mobile - BlockDraggable - Set isEditingText to false and update it with the initialization of the AztecView listener * Mobile - AztecView - Move notifyInputChange to the focus/blur functions within AztecInputState, to fix an issue where these are called directly. * [RNMobile] Disable dragging when a screen reader is enabled (#40852) * [RNMobile] Allow dragging from block toolbar (#40886) * Enable dragging from block toolbar * Ensure root block is dragged for nested blocks * Add draggingClientId prop to BlockDraggable With this change the `BlockDraggable` component might receive two different client ids, one is the client id of the block where the component is rendered, and the other (which is optional) is used for identifying the block to be dragged. * Set dragging always enabled for block toolbar The block toolbar is only visible when the block is selected so it's safe to allow dragging in all cases, as it won't affect other UI elements. * Update dragging enabled calculation In order to prevent issues related to disabling the long-press gesture in elements within the block UI, the logic for enabling the dragging has been updated. Now it will enabled in the following cases: - The block doesn't have inner blocks. This applies to root blocks and nested blocks without further nested blocks. - Blocks with nested blocks if the block is selected. - Blocks with nested blocks if none of the nested blocks is selected. * Update Podfile.lock * Update react-native-editor changelog * Fix text block query locator in Android E2E tests * Fix Cover block E2E test * Fix Spacer block E2E test Co-authored-by: Gerardo Pacheco <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes wordpress-mobile/gutenberg-mobile#4601
Gutenberg Mobile PR-> wordpress-mobile/gutenberg-mobile#4611Description
This PR includes the following changes:
Block List context was moved into its own file to expand its functionality:
scrollRef(before it was on the top level).blocksLayoutsto store the blocksonLayoutdata.updateBlocksLayoutsto be able to update theblocksLayoutsdata.useBlockListContextto be able to access the context.findByRootIdto look for a block's data by root id.deleteByClientIdto delete a block's data fromblocksLayouts.BlockListItemCellnew component:This is now a wrapper for every item of the block list FlatList, it will use
onLayoutto store the block's size and coordinates into the Block list context:blocksLayouts. This component is set usingCellRendererComponentthis is needed in order to get the correctx,ycoordinates. Trying to get this data withinrenderItemwould return0for thexandycoordinates.An example of the data stored in
blocksLayouts:Expand
{ "c8965391-4798-4ba5-b3b3-e51be1f84a33": { "clientId": "c8965391-4798-4ba5-b3b3-e51be1f84a33", "width": 390, "height": 241.33334350585938, "x": 0, "y": 164, "innerBlocks": { "b6334622-c13c-4a9b-bafc-5e467592b1b2": { "clientId": "b6334622-c13c-4a9b-bafc-5e467592b1b2", "rootClientId": "c8965391-4798-4ba5-b3b3-e51be1f84a33", "width": 187, "height": 209.33334350585938, "x": 187, "y": 0, "innerBlocks": {} }, "054e6463-1c6c-49fe-8d92-92cd2259ee4b": { "clientId": "054e6463-1c6c-49fe-8d92-92cd2259ee4b", "rootClientId": "c8965391-4798-4ba5-b3b3-e51be1f84a33", "width": 187, "height": 209.33334350585938, "x": 0, "y": 0, "innerBlocks": {} } } }, "ad9b6a24-09ae-48c8-87a1-93976485b0ee": { "clientId": "ad9b6a24-09ae-48c8-87a1-93976485b0ee", "width": 390, "height": 81, "x": 0, "y": 83, "innerBlocks": {} }, "f860cafe-d9f2-4916-a161-d96855f9c06c": { "clientId": "f860cafe-d9f2-4916-a161-d96855f9c06c", "width": 390, "height": 56.666656494140625, "x": 0, "y": 405.3333435058594, "innerBlocks": {} } }Testing Instructions
Manual testing
console.logto include the blocksconsole.logScreenshots
N/A
Types of changes
New feature
Checklist:
*.native.jsfiles for terms that need renaming or removal).