Gutenberg-mobile: FloatingToolbar ideal version

Created on 7 Oct 2019  路  15Comments  路  Source: wordpress-mobile/gutenberg-mobile

This is the follow-up task for issue: https://github.com/wordpress-mobile/gutenberg-mobile/issues/1413

We should update FloatingToolbar to gain the Floating behavior, here's the comparison between temp & ideal solutions:

FloatingToolbar

cc: @iamthomasbishop

InnerBlocks

All 15 comments

After spending some time looking at this issue here's what I found so far:

iOS

iOS is the easier one here. For iOS, you can add a custom view "OverflowView" to the view hierarchy and override (UIView *)hitTest:(CGPoint)point withEvent:(UIEvent *)event with a custom implementation. If you normalize the point in the window with:
CGPoint normalizedPoint = [self convertPoint:point toView:nil];

Then if you compare that to the normalized frame:

CGRect adjsutedFrame = subview.frame;
adjsutedFrame.origin = [parent convertPoint:subview.frame.origin toView:nil];

Then you can traverse the view hierarchy and respond appropriately to the touch. The end result looks something like this:

- (UIView *)hitTest:(CGPoint)point withEvent:(UIEvent *)event {

    CGPoint normalizedPoint = [self convertPoint:point toView:nil];
    UIView *successfulHit = [self checkHitTest:normalizedPoint withEvent:event inView:self];

    if(successfulHit) {
        return successfulHit;
    }

    return [super hitTest:point withEvent:event];
}

- (UIView *)checkHitTest:(CGPoint)point withEvent:(UIEvent *)event inView:(UIView *)parent {

    for (UIView *subview in parent.subviews) {

        CGRect adjsutedFrame = subview.frame;
        adjsutedFrame.origin = [parent convertPoint:subview.frame.origin toView:nil];

        if (CGRectContainsPoint(adjsutedFrame, point)) {
            NSLog(@"HIT!");
            return subview;
        } else {
            return [self checkHitTest:point withEvent:event inView:subview];
        }
    }

    return nil;
}

I was also able to get this to work in a less ideal way in iOS with no code changes but by increasing the hitslop on the JS component.

Android

Android has been a little more problematic.

As a refresher for handling touch events in Android, I would recommend looking at

I set up a simplified example of this problem in Expo
screenshot-1581431743233

In the above image, the Red and Orange boxes represent a block, and the Green box is the floating toolbar.
I then made the following adjustments

  • On the FlatList I added a CellRendererComponent to return a View subclass OverflowView
  • I adjusted the OverflowView and subviews to have an increased hitslop down to the view that housed the Floating toolbar

With these changes, if you entered the inspector and tapped on the Floating Toolbar the correct Block would be highlighted but no clicks would process.

Using the example above: If I tapped on the Floatingtoolbar (green box) and had break points in dispatchTouchEvent() and onInterceptTouchEvent() my expectation would be that the Orange block would get these events. In actuality, the Red box gets these events and Orange never fires. So somewhere in the line, the events are being swallowed.

I did find this PR https://github.com/facebook/react-native/pull/26374 which seems to address part of our problem. However, trying this locally doesn't seem to fully resolve the issue.

I think by overriding ReactViewGroup to parse through the views in an order based on z-index and having the views follow the Android touch detection pattern in that order would allow us to capture the taps and adjust accordingly.

Will need to come back to this later

@chipsnyder

iOS is the easier one here. For iOS, you can add a custom view "OverflowView" to the view hierarchy and override (UIView *)hitTest:(CGPoint)point withEvent:(UIEvent *)event with a custom implementation. If you normalize the point in the window with:
CGPoint normalizedPoint = [self convertPoint:point toView:nil];

Do we really need a native change to make it works on iOS? As far as I remember we were able to implement that on iOS and only Android was the problem, but i'm not 100% sure :)

@dratwas Correct me if I am wrong but I think it was still a kind of workaround. AFAIR, the solution was putting the top most block's FloatingToolbar in the parent level instead of the block level. It worked on iOS but it also introduced an exception in the code design which we don't prefer in the long term.

@pinarol I think it was the Android case. I created a simple snack with https://snack.expo.io/rJiqFLbXU and it seems to work on iOS. Also, I decided to check that in our app since it could work differently in the snack.

To reproduce it please open gutenberg/packages/block-editor/src/components/block-list/block-mobile-floating-toolbar.native.scss and add

    position:absolute;
    top: -50;

to the .floatingToolbar styles

Should look like

.floatingToolbar {
    background-color: $dark-gray-500;
    margin: auto;
    min-width: 100;
    max-height: $floating-toolbar-height;
    border-radius: 22px;
    flex-direction: row;
    z-index: 100;
    height: $floating-toolbar-height;
    align-items: center;
    justify-content: center;
    align-self: center;
    margin-bottom: 8px;
    position:absolute;
    top: -50;
}

The floating toolbar is positioned absolute and all is clickable inside w/o any native change.

@dratwas You're right I had forgotten I got that working in my Snack with no native changes. But I wasn't able to get the correct highlights in Android without doing some increase on the hitslop. Once I started adding the hitslop spacing, it would still work, but I would probably need some native code still.

but I would probably need some native code still.

In iOS or Android? I agree that we need some native changes on Android but not sure why we need these on iOS.

Oh sorry yeah I didn't really complete that thought. I meant in both but only if hitslop is required for the end Android Implementation. However, we could only add that if the platform is Android. The issue I was running into with iOS and an increased hitslop then the view below would get the taps on iOS when next to the floating toolbar. That all depends on where Android ends up though.

I dug into history to be sure. Unfortunately, it wasn't working on iOS without the workaround at RN side.
Please refer to this comment.

And locate this commit under it:

Screen Shot 2020-02-12 at 15 22 29

Only after that workaround, we had a working result for iOS.

Then there鈥檚 a new comment saying that workaround didn鈥檛 work for Android: https://github.com/WordPress/gutenberg/pull/17399#issuecomment-535491166

I'd like to underline one thing to avoid confusions, the unreceived touch issue on the FloatingBar only manifests itself at the top most block in the FlatList.

I'll drop a rather more primitive version of the iOS side solution I had in my mind for documentation purposes:

Assuming:

  • OverflowView is the block container view
  • OverflowView inherits RCTView so that styling settings can be applied with no extra development

on JS side, pass the tag of FloatingToolbar instance explicitly:

<OverflowView overflowingViewTag={ this.state.floatingToolbarTag } >
   <FloatingToolbar onRef={ (ref) => { this.setState({ floatingToolbarTag: ReactNative.findNodeHandle(ref)}) } }/>
</OverflowView>

on native side, inside OverflowView class, override hitTest and just check for FloatingToolbar intercepts with the touch point:

- (UIView *)hitTest:(CGPoint)point withEvent:(UIEvent *)event {

   UIView *floatingToolbar = [uiManager viewForReactTag: overflowingViewTag];
   //detect if the touch is on floatingToolbar, if so, return it

Hey, @chipsnyder I wanted to point to one thing. Please test your solution for the group block as well since the solution that I posted above doesn't work for the first block in the group block. It happens because the floating toolbar is rendered outside the Group and is covered by the block that is above the Group.
Screenshot 2020-02-14 at 12 26 33

@dratwas Good call out. My solution does work for Group Block, which is the primary way I was testing it.

@chipsnyder could you push your changes to a remote branch and link them here?

@chipsnyder could you push your changes to a remote branch and link them here?

Yup no problem!

The work from where I left off is available in these branches:
gutenberg-mobile issue/1413-floatingToolbar
gutenberg issue/1414-floatingToolbar-react

As a bit of an update, we are still struggling to get the positioning aspects of the toolbar to cooperate in a scalable way, so per some discussion amongst the team (@pinarol @chipsnyder), we are going take a slightly different approach.

  • Positioning will move to become "fixed" to the top of the block toolbar, centered, with ~8px space in between.
  • The toolbar will remain fixed in this position while the user is scrolling.
  • Bonus points if we can do the following: When the user taps on the breadcrumbs, scroll the user to the selected block.
  • Contents of the toolbar aren't changing at all. Still showing the up icon, separator, and breadcrumbs (parent block icon, nesting icon, selected block icon, and name of selected block).

Here is a rough sketch to demonstrate:

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Tug picture Tug  路  3Comments

chipsnyder picture chipsnyder  路  3Comments

mkevins picture mkevins  路  3Comments

maxme picture maxme  路  3Comments

designsimply picture designsimply  路  4Comments