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

improvement: avoid two taps requirement for android #51

Conversation

lucianomlima
Copy link
Contributor

Description

This PR adds keyboardShouldPersistTaps with handled as default value to improve usability in some cases like having focus in an input, but needs two taps to interact with pressable components like buttons, sliders and switchs.

To clarify the issue, I record 2 videos:

without keyboardShouldPersistTaps

We needs two taps, 1 for keyboard dismiss and another one to interact with pressable.

without-keyboardShouldPersistTaps.mp4

with keyboardShouldPersistTaps as handled

Here we need only 1 tap to interact with pressable.

with-keyboardShouldPersistTaps-as-handled.mp4

Related Issues

resolves #29

Tests

Don't need new tests1, only a snapshot update. But to tests in your computer, you can use the MansoryExample App.tsx patch below:

MasonryExample/src/App.tsx.patch
diff --git a/MasonryExample/src/App.tsx b/MasonryExample/src/App.tsx
index ef22d5e..254921b 100644
--- a/MasonryExample/src/App.tsx
+++ b/MasonryExample/src/App.tsx
@@ -1,15 +1,19 @@
 import {
+  Alert,
   Image,
+  Pressable,
   SafeAreaView,
   StatusBar,
   StyleProp,
+  StyleSheet,
   Text,
+  TextInput,
   View,
   ViewStyle,
   useColorScheme,
 } from 'react-native';
 // eslint-disable-next-line @typescript-eslint/no-use-before-define
-import React, {FC, ReactElement, useMemo} from 'react';
+import React, {useMemo, useState} from 'react';
 
 import {Colors} from 'react-native/Libraries/NewAppScreen';
 import MasonryList from '@react-native-seoul/masonry-list';
@@ -19,6 +23,7 @@ interface Furniture {
   id: string;
   imgURL: string;
   text: string;
+  pressable?: boolean;
 }
 
 const data: Furniture[] = [
@@ -39,12 +44,14 @@ const data: Furniture[] = [
     imgURL:
       'https://hips.hearstapps.com/hmg-prod.s3.amazonaws.com/images/leverette-fabric-queen-upholstered-platform-bed-1594829293.jpg',
     text: 'Leverette Upholstered Platform Bed',
+    pressable: true,
   },
   {
     id: 'id126',
     imgURL:
       'https://hips.hearstapps.com/hmg-prod.s3.amazonaws.com/images/briget-side-table-1582143245.jpg?crop=1.00xw:0.770xh;0,0.129xh&resize=768:*',
     text: 'Briget Accent Table',
+    pressable: true,
   },
   {
     id: 'id127',
@@ -63,6 +70,7 @@ const data: Furniture[] = [
     imgURL:
       'https://hips.hearstapps.com/hmg-prod.s3.amazonaws.com/images/goodee-ecobirdy-charlie-chairs-1594834221.jpg?crop=1xw:1xh;center,top&resize=768:*',
     text: 'Ecobirdy Charlie Chair',
+    pressable: true,
   },
   {
     id: 'id130',
@@ -168,15 +176,32 @@ const data: Furniture[] = [
   },
 ];
 
-const FurnitureCard: FC<{item: Furniture; style: StyleProp<ViewStyle>}> = ({
+const FurnitureCard = ({
   item,
   style,
-}) => {
+}: {
+  item: Furniture;
+  style: StyleProp<ViewStyle>;
+}): JSX.Element => {
   const randomBool = useMemo(() => Math.random() < 0.5, []);
   const {theme} = useTheme();
+  const Wrapper = item.pressable ? Pressable : View;
 
   return (
-    <View key={item.id} style={[{marginTop: 12, flex: 1}, style]}>
+    <Wrapper
+      key={item.id}
+      onPress={() =>
+        Alert.alert('Item pressed:', item.text, undefined, {cancelable: true})
+      }
+      style={[
+        {
+          marginTop: 12,
+          flex: 1,
+          backgroundColor: item.pressable ? '#DFDFDF' : '#FFF',
+        },
+        style,
+      ]}
+    >
       <Image
         source={{uri: item.imgURL}}
         style={{
@@ -188,16 +213,18 @@ const FurnitureCard: FC<{item: Furniture; style: StyleProp<ViewStyle>}> = ({
       <Text
         style={{
           marginTop: 8,
-          color: theme.text,
+          color: item.pressable ? 'blue' : theme.text,
+          textDecorationLine: item.pressable ? 'underline' : 'none',
         }}
       >
         {item.text}
       </Text>
-    </View>
+    </Wrapper>
   );
 };
 
-const App: FC = () => {
+const App = (): JSX.Element => {
+  const [text, onChangeText] = useState('');
   const isDarkMode = useColorScheme() === 'dark';
 
   const backgroundStyle = {
@@ -205,13 +232,7 @@ const App: FC = () => {
     flex: 1,
   };
 
-  const renderItem = ({
-    item,
-    i,
-  }: {
-    item: Furniture;
-    i: number;
-  }): ReactElement => {
+  const renderItem = ({item, i}: {item: Furniture; i: number}): JSX.Element => {
     return (
       <FurnitureCard item={item} style={{marginLeft: i % 2 === 0 ? 0 : 12}} />
     );
@@ -221,8 +242,10 @@ const App: FC = () => {
     <SafeAreaView style={backgroundStyle}>
       <StatusBar barStyle={isDarkMode ? 'light-content' : 'dark-content'} />
       <MasonryList
+        removeClippedSubviews
         keyExtractor={(item: Furniture): string => item.id}
-        ListHeaderComponent={<View />}
+        keyboardDismissMode="on-drag"
+        keyboardShouldPersistTaps="handled"
         contentContainerStyle={{
           paddingHorizontal: 24,
           alignSelf: 'stretch',
@@ -231,8 +254,26 @@ const App: FC = () => {
         data={data}
         renderItem={renderItem}
       />
+      <TextInput
+        cursorColor="#888"
+        onChangeText={onChangeText}
+        placeholder="focus to show keyboard"
+        style={styles.textInput}
+        value={text}
+      />
     </SafeAreaView>
   );
 };
 
+const styles = StyleSheet.create({
+  textInput: {
+    height: 40,
+    marginHorizontal: 24,
+    marginVertical: 12,
+    borderColor: '#888',
+    borderRadius: 8,
+    borderWidth: 1,
+    padding: 12,
+  },
+});
 export default App;

Checklist

Before you create this PR confirms that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • Run yarn lint && yarn tsc
  • Run yarn test or yarn test -u if you need to update snapshot.
  • I am willing to follow-up on review comments in a timely manner.

Footnotes

  1. I update some tests that don't requires to be async.

@hyochan hyochan added the 🍗 enhancement New feature or request label Oct 10, 2022
Copy link
Owner

@hyochan hyochan left a comment

Choose a reason for hiding this comment

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

Thanks so much for putting the defaultkeyboardShouldPersistTaps props.
I am not sure why this doesn't default in react-native 🤔

@hyochan hyochan merged commit 044d249 into hyochan:main Oct 10, 2022
@lucianomlima lucianomlima deleted the improvement/avoid-two-taps-requirement-for-android branch October 10, 2022 05:53
@lucianomlima
Copy link
Contributor Author

lucianomlima commented Oct 13, 2022

@hyochan sorry to get into this, but this year the participation on Hacktoberfest changed a little bit.

YOUR PR/MRS MUST BE IN A REPO TAGGED WITH THE “HACKTOBERFEST” TOPIC, OR HAVE THE “HACKTOBERFEST-ACCEPTED” LABEL.

So, can you please add the hacktoberfest-accepted to the this PR to marked as participating? 🙏

@hyochan
Copy link
Owner

hyochan commented Oct 13, 2022

@hyochan sorry to get into this, but this year the participation on Hacktoberfest changed a little bit.

YOUR PR/MRS MUST BE IN A REPO TAGGED WITH THE “HACKTOBERFEST” TOPIC, OR HAVE THE “HACKTOBERFEST-ACCEPTED” LABEL.
So, can you please add the hacktoberfest-accepted to the this PR to marked as participating? 🙏

Just done it! Hope it helps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add keyboardShouldPersistTaps="handled"
2 participants