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

[Hold for Payment 3 August] Edit comment - Mouse scroll doesn't scroll large messages in edit box #4132

Closed
isagoico opened this issue Jul 17, 2021 · 19 comments
Assignees
Labels
Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@isagoico
Copy link

isagoico commented Jul 17, 2021

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


Action Performed:

  1. Log in to e.cash and navigate to a conversation.
  2. Send a long message (long enough so the edit box has a scrollbar)
  3. Edit the message and try to scroll the long text via mouse scroll

Expected Result:

Mouse scroll should work on edit box if the user is hovering over it.

Actual Result:

Mouse scrolls the whole conversation making the user unable to scroll the edit box via mouse scroller.

Workaround:

Using the arrow keys scrolls the message.

Platform:

Where is this issue occurring?

Web ✔️
iOS
Android
Desktop App
Mobile Web

Version Number: 1.0.79-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos:

Grabando.197.mp4

https://www.upwork.com/jobs/~0188b70c9ae217c170

View all open jobs on Upwork


From @cead22 https://expensify.slack.com/archives/C01GTK53T8Q/p1626382794346600

When editing a long message, you can't scroll the editable area via mouse scroll. Hitting the down arrow will make the area scroll when you reach a line that's below the lower bound

@MelvinBot
Copy link

Triggered auto assignment to @sketchydroide (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@sketchydroide
Copy link
Contributor

This sounds like a good external.

@sketchydroide sketchydroide added the External Added to denote the issue can be worked on by a contributor label Jul 19, 2021
@MelvinBot
Copy link

Triggered auto assignment to @laurenreidexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@sketchydroide
Copy link
Contributor

sketchydroide commented Jul 19, 2021

Also setting it as weekly, there is a workaround, and it does not stop functionality it's just anoying, as mouse scrolling would be an expected behaviour.

@laurenreidexpensify
Copy link
Contributor

@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 19, 2021
@MelvinBot
Copy link

Triggered auto assignment to @deetergp (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@ahmedmustafamo
Copy link

@isagoico I want to run the app as local
it works fine. But when I login with email and password it redirects me on the live app
may I know why ?
I want to help with this issue

@rdjuric
Copy link
Contributor

rdjuric commented Jul 19, 2021

This happens because we're adding a listener to our InvertedFlatList wheel events in the web.
https://github.com/Expensify/Expensify.cash/blob/fc965f68d308167d19d00a29b91e85ca33707882/src/components/InvertedFlatList/index.js#L35-L37
We need this listener as adding the inverted={true} prop to a FlatList will also invert the wheel direction. This is being fixed on react-native-web 0.19 and we will no longer need this listener.

I don't feel like this issue needs an immediate fix, so I would suggest waiting for the 0.19 update. We could add a workaround (like removing the listener if a ReportActionItemMessageEdit is focused), but I feel like this would add unnecessary complexity to the code.

Curious to hear what you think @deetergp

@helaoutar
Copy link
Contributor

helaoutar commented Jul 19, 2021

Hi,
I'm thinking about intercepting the wheel event on the text input, then stop it from being passed up (propagated) to the InvertedFlatList.
I tried it out, and it seems to be working.

Screen.Recording.2021-07-19.at.16.28.02.4.mov

@ahmedmustafamo
Copy link

Hi,
I'm thinking about intercepting the wheel event on the text input, then stop it from being passed up (propagated) to the InvertedFlatList.
I tried it out, and it seems to be working.
Screen.Recording.2021-07-19.at.16.28.02.4.mov

Good work

@deetergp
Copy link
Contributor

@helaoutar I think your suggestion sounds good, let's do it.

@helaoutar
Copy link
Contributor

@helaoutar I think your suggestion sounds good, let's do it.

@deetergp

The only issue I found with this is that as soon as the pointer is within the text input, the page stops scrolling (which makes sense, since the text input prevents the wheel event from being passed up to the page)

This becomes problematic in the following scenario:

420.mov

See how since the pointer is initially at the bottom of the page, the moment I scroll down, the pointer is stuck within the text input, and I have to get it out of it to scroll the page.

Hope that makes sense.

@parasharrajat
Copy link
Member

@helaoutar Only override wheel when input is focused.

@rdjuric
Copy link
Contributor

rdjuric commented Jul 22, 2021

(like removing the listener if a ReportActionItemMessageEdit is focused)

That's what I proposed, but it's okay. You should definitely detach the listener only when the input is focused @helaoutar

@helaoutar
Copy link
Contributor

Can you please hire me for this one so that I can start working on it?

@laurenreidexpensify
Copy link
Contributor

@helaoutar done - please start on the PR! thanks

@laurenreidexpensify laurenreidexpensify removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 26, 2021
@laurenreidexpensify laurenreidexpensify changed the title Edit comment - Mouse scroll doesn't scroll large messages in edit box [Hold for Payment 3 August] Edit comment - Mouse scroll doesn't scroll large messages in edit box Jul 30, 2021
@MelvinBot
Copy link

@deetergp, @helaoutar, @laurenreidexpensify it looks like no one is assigned to work on this job.
Please double the price or add a comment stating why the job isn't being doubled.

@laurenreidexpensify
Copy link
Contributor

Ignore - this is a false positive, as we're closing this tomorrow when regression testing period is over

@laurenreidexpensify
Copy link
Contributor

Paid @helaoutar 👍🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests

9 participants