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

Fix scroll interactions #2631

Closed
wants to merge 38 commits into from
Closed

Fix scroll interactions #2631

wants to merge 38 commits into from

Conversation

m-bert
Copy link
Contributor

@m-bert m-bert commented Oct 12, 2023

Description

During development of web version we initially decided to set touchAction: none to all gesture handlers' views in order to avoid scrolling while gestures like pan are active. Now it turns out that because of this decision, components like FlatList or ScrollView may become useless when combined with some handlers (or, like in the issue mentioned below - Swipeable components).

This PR aims to fix that problem by replacing touchAction: none with e.preventDefault().

Fixes #2617

Test plan

Tested on:

  • Draggable example
  • Transformation example (with ScrollView and LoremIpsum)
Modified code from issue
import React from 'react';
import { View, Text } from 'react-native';
import { FlatList, GestureHandlerRootView } from 'react-native-gesture-handler';
import Swipeable from 'react-native-gesture-handler/Swipeable';

export default function Home() {
  type ItemProps = {
    item: {
      text: string;
    };
  };

  const data = Array.from({ length: 50 }, (_, i) => ({
    id: i,
    text: `Item ${i}`,
  }));

  const Item = ({ item }: ItemProps) => {
    return (
      <View style={{ margin: 10 }}>
        <Swipeable
          renderRightActions={() => (
            <View
              style={{
                justifyContent: 'center',
              }}>
              <Text style={{ color: 'white', textAlign: 'center' }}>
                Delete
              </Text>
            </View>
          )}>
          <View
            style={{
              height: 50,
              backgroundColor: 'white',
              justifyContent: 'center',
            }}>
            <Text>{item.text}</Text>
          </View>
        </Swipeable>
      </View>
    );
  };

  return (
    <GestureHandlerRootView>
      <FlatList
        data={data}
        keyExtractor={(item) => item.id.toString()}
        renderItem={({ item }) => <Item item={item} />}
        style={{ maxHeight: 400 }}
      />
    </GestureHandlerRootView>
  );
}

src/web/tools/InteractionManager.ts Outdated Show resolved Hide resolved
src/web/handlers/PanGestureHandler.ts Outdated Show resolved Hide resolved
src/web/handlers/PanGestureHandler.ts Outdated Show resolved Hide resolved
src/web/handlers/PanGestureHandler.ts Outdated Show resolved Hide resolved
src/web/handlers/PanGestureHandler.ts Outdated Show resolved Hide resolved
src/web/tools/GestureHandlerOrchestrator.ts Show resolved Hide resolved
@m-bert m-bert marked this pull request as ready for review October 16, 2023 08:15
src/web/handlers/GestureHandler.ts Outdated Show resolved Hide resolved
src/web/handlers/PanGestureHandler.ts Outdated Show resolved Hide resolved
src/web/handlers/PanGestureHandler.ts Outdated Show resolved Hide resolved
src/web/handlers/PanGestureHandler.ts Outdated Show resolved Hide resolved
src/web/handlers/PanGestureHandler.ts Outdated Show resolved Hide resolved
@j-piasecki
Copy link
Member

Looks like there still are some things left to iron out

screen-20231031-095931.2.mov
screen-20231031-100142.2.mov
screen-20231031-100455.2.mov

All captured on Android 13, Chrome 118.0.5993.111

@m-bert
Copy link
Contributor Author

m-bert commented Nov 6, 2023

@j-piasecki I agree that there are some problems that should be fixed. However, I've checked the one with swipeable on main and it seems that it is already there, even without this PR. Could you please check if that's true?

@j-piasecki
Copy link
Member

However, I've checked the one with swipeable on main and it seems that it is already there, even without this PR. Could you please check if that's true?

Works fine for me on the current main (8d3d5e0)

screen-20231107-091954.2.mov

@GuestInCorle
Copy link

@m-bert What's blocking this pull request from getting a review?

@j-piasecki
Copy link
Member

iOS doesn't seem to work at all yet - all handlers are running simultaneously with scrolling the page and navigation gestures

@m-bert
Copy link
Contributor Author

m-bert commented Jan 2, 2024

Hi @GuestInCorle!

Problems that @j-piasecki described seem to affect both platforms, Android and iOS.

I'm not sure about the navigation part and I have yet to check that. However, most of the issues seem to originate from default minDistance in PanGestureHandler (which right now is set to 15px, see this line). By manipulating this value I was able to achieve what I think is the solution that we are looking for.

@m-bert m-bert mentioned this pull request Mar 4, 2024
m-bert added a commit that referenced this pull request Mar 5, 2024
## Description

This PR adds new prop to gesture handlers - `touchAction`. 

While working on [fix scroll interactions PR](#2631) we've found it difficult to remove `touch-action: none;` and ensure correct behavior of handlers with scroll, especially with swipeable components. What made it even harder were differences between platforms. 

We decided to kill two birds with one stone - give our users possibility to manipulate `touchAction`, which will also make it easier for us to properly handle scroll.

## Test plan

Tested on example app.
@m-bert
Copy link
Contributor Author

m-bert commented Mar 7, 2024

This PR was superseded by #2788, followed by #2794.

@m-bert m-bert closed this Mar 7, 2024
@m-bert m-bert deleted the @mbert/fix-scroll-interactions branch April 8, 2024 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FlatList component does not scroll vertically on touch in mobile responsive mode
3 participants