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

[$500] Chat - Keyboard closes when swiping down the multiline text in the composer #31376

Closed
2 of 6 tasks
lanitochka17 opened this issue Nov 15, 2023 · 19 comments
Closed
2 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@lanitochka17
Copy link

lanitochka17 commented Nov 15, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 1.3.99-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Launch New Expensify app
  2. Go to any chat
  3. Enter many lines of message
  4. Expand the composer
  5. Swipe down within the composer to scroll up to the first line

Expected Result:

Keyboard will not close when swiping down the multiline texts in the composer

Actual Result:

Keyboard closes when swiping down the multiline texts in the composer, especially when the swipe is quick

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6277655_1700058942794.Screen_Recording_20231115_154256_New_Expensify.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01f22d16b807352fe4
  • Upwork Job ID: 1724807755655684096
  • Last Price Increase: 2023-11-15
@lanitochka17 lanitochka17 added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 15, 2023
@melvin-bot melvin-bot bot changed the title Chat - Keyboard closes when swiping down the multiline text in the composer [$500] Chat - Keyboard closes when swiping down the multiline text in the composer Nov 15, 2023
Copy link

melvin-bot bot commented Nov 15, 2023

Triggered auto assignment to @slafortune (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

Copy link

melvin-bot bot commented Nov 15, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01f22d16b807352fe4

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 15, 2023
Copy link

melvin-bot bot commented Nov 15, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

Copy link

melvin-bot bot commented Nov 15, 2023

Triggered auto assignment to Contributor-plus team member for initial proposal review - @hoangzinh (External)

@manlaig
Copy link

manlaig commented Nov 15, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

Keyboard dismissed when swiping down

What is the root cause of that problem?

The onSwipeDown handler is dismissing the Keyboard here:

<SwipeableView onSwipeDown={Keyboard.dismiss}>

What changes do you think we should make in order to solve the problem?

Remove the onSwipeDown={Keyboard.dismiss}
Furthermore, we can remove the whole line 94 since we don't need the SwipeableView component to dismiss the keyboard.

Result

iOS

RPReplay_Final1700066217.MOV

Android

402313177_7301941956491877_6912841167996883366_n.mp4

What alternative solutions did you explore? (Optional)

@manlaig
Copy link

manlaig commented Nov 15, 2023

Updated proposal

@manlaig
Copy link

manlaig commented Nov 15, 2023

Updated proposal to include a screen recording

@manlaig
Copy link

manlaig commented Nov 15, 2023

Updated proposal to add Android screen recording

@hoangzinh
Copy link
Contributor

@manlaig Thanks for your proposal. Could you check git blame and find the reason why we set onSwipeDown={Keyboard.dismiss} previously?

@manlaig
Copy link

manlaig commented Nov 17, 2023

@hoangzinh Looks like it was added in the commit that originally added the ReportFooter.js file here:

150cad6#diff-6cbc88ae1e31fd9d7bba715c94c947a8bcb1d2cd3cfcfb7368755994d40b2b02R87

Before that, it was part of the ReportScreen.js where it was first introduced over 2 years ago in this commit:

47d99be#diff-dd4bfa50713397aca8cb40145317936ab0affe19abc2a9c24d6c8849ed75dc9bR139

@manlaig
Copy link

manlaig commented Nov 17, 2023

Actually, it originally came from ReportView.js file that was refactored into ReportScreen.js file in that commit 2 years ago. The first commit adding the SwipeableView component is this commit:

956eb7a#diff-0fde357effe9cfca0e46ae05dde144722f716890ac7bb5b1d233de2cae5e696cR29

@ykhateeb
Copy link

ykhateeb commented Nov 18, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

Swiping down in The Composer during the multiline state causing the keyboard to close

What is the root cause of that problem?

The root cause of this issue is that SwipeableView component catches all swiping events that are occurring under it in the Capture phase, which is causing onMoveShouldSetPanResponderCapture method to be called in scrolling the multiline text in the Composer.

onMoveShouldSetPanResponderCapture: (_event, gestureState) => {

Another possible case that causes this issue is when selecting a text in the Composer and swiping down as in the video below.

Screen.Recording.2023-11-18.at.1.37.09.AM.mov

In general, any swiping down event(more than the distance of COMPOSER_MAX_HEIGHT) in the Composer, will cause the keyboard to close.

What changes do you think we should make in order to solve the problem?

We have first to filter Composer movements events in onMoveShouldSetPanResponderCapture by adding the following check:

const isTextInputEvent =
  event.target?.viewConfig?.uiViewClassName.includes('TextInput');
PanResponder.create({
  // The PanResponder gets focus only when the y-axis movement is over minimumPixelDistance & swipe direction is downwards
  // eslint-disable-next-line @typescript-eslint/naming-convention
  onMoveShouldSetPanResponderCapture: (event, gestureState) => {
    //check if the event is from a TextInput
    const isTextInputEvent =
      event.target?.viewConfig?.uiViewClassName.includes('TextInput');

    if (
      gestureState.dy - oldYRef.current > 0 &&
      gestureState.dy > minimumPixelDistance &&
      //does not apply if the event is from a TextInput
      !isTextInputEvent
    ) {
      return true;
    }
    oldYRef.current = gestureState.dy;
    return false;
  },

  // Calls the callback when the swipe down is released; after the completion of the gesture
  onPanResponderRelease: onSwipeDown,
});

After this change, we will have two main areas the Composer (in white border) which will not respond to swipe down event and the other components(in red borders) which will respond to swipe down event, as we can see above the Composer there is no enough space for the user can touch, so I suggest to move the margin-top above the Composer to be a part of it to make swipe easer from up and down.

Untitled-2023-09-18-0749 (2)

What alternative solutions did you explore? (Optional)

Copy link

melvin-bot bot commented Nov 18, 2023

📣 @ykhateeb! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@hoangzinh
Copy link
Contributor

hoangzinh commented Nov 18, 2023

@slafortune It looks like it's not a bug but it's expected behavior since this change #2074.
Basically:

  • If user do a short scroll (shorter than the composer height) then there is not thing happen, Keyboard won't be dismissed
  • But if user do a long scroll (longer than the composer height), then Keyboard will be dismissed
Screen.Recording.2023-11-18.at.08.21.29.mov

Just wanna confirm if you want to cont with expectation of this issue or close this issue. Thanks.

@ykhateeb
Copy link

ykhateeb commented Nov 18, 2023

Contributor details
Your Expensify account email: ykhateeb93@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0181d1beee862a16fa

Copy link

melvin-bot bot commented Nov 18, 2023

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

@ykhateeb
Copy link

Contributor details
Your Expensify account email: ykhateeb93@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0181d1beee862a16fa

Copy link

melvin-bot bot commented Nov 18, 2023

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

@melvin-bot melvin-bot bot added the Overdue label Nov 20, 2023
@slafortune
Copy link
Contributor

Ah! Thanks for the details @hoangzinh, yes - I agree this is intentional.

@melvin-bot melvin-bot bot removed the Overdue label Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

5 participants