Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.51k stars 2.87k forks source link

[$250] Pressing space in the participant's split amount input goes to user's profile page. #48605

Closed m-natarajan closed 1 month ago

m-natarajan commented 1 month ago

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: 9.0.29-6 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: @jayeshmangwani Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1725484726223549

Action Performed:

Action Performed:

  1. Open the app and go to any group chat
  2. Press + -> Split expense -> Enter amount -> Next
  3. Edit any participant's amount and press Space

Expected Result:

Pressing space should have done nothing, just like any other non-number key press

Actual Result:

Pressing space in split amount input goes to user's profile

Workaround:

unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/8c418dbf-2d57-44ef-89aa-5c3c96ecf12b

https://github.com/user-attachments/assets/2401755b-3fe3-4dcf-b70c-da5f5998717e

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021831765734761522413
  • Upwork Job ID: 1831765734761522413
  • Last Price Increase: 2024-09-05
Issue OwnerCurrent Issue Owner: @getusha
melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @garrettmknight (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

mkzie2 commented 1 month ago

Edited by proposal-police: This proposal was edited at 2024-09-05 05:07:41 UTC.

Proposal

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

Pressing space in split amount input goes to user's profile

What is the root cause of that problem?

A Pressable with Button role will be rendered as button tag. A DOM button will trigger onclick on Spacebar key by default. Reference: https://github.com/necolas/react-native-web/issues/2560#issuecomment-1639812931. You can try it yourself.

When we press the spacebar in the amount tex input, the keyboard event will propagate to the parent Pressable then trigger the participant item's onPress and eventually open participant details page.

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

The problem here are:

Fortunately, we can take advantage of onKeyDownCapture event which captures all key down events, independent of browser behavior.

  1. Introduce isSpaceKeyDownOnInputRef to detect the Spacebar keydown event in BaseListItem
  2. Specify onKeyDownCapture:
if (e && 'key' in e && e.key === ' ') {
    isSpaceKeyDownOnInputRef.current = true;
}
  1. Introduce shouldPreventSpaceKeySubmit prop in BaseListItem, and early return in onPress just like shouldPreventEnterKeySubmit:

https://github.com/Expensify/App/blob/651c9e04a7f636fa1b1ca4986e926cb73de71107/src/components/SelectionList/BaseListItem.tsx#L83-L85

if (shouldPreventSpaceKeySubmit && isSpaceKeyDownOnInputRef.current) {
    isSpaceKeyDownOnInputRef.current = false;
    return;
}
  1. Toggle shouldPreventSpaceKeySubmit in BaseSelectionList;

https://github.com/Expensify/App/blob/41e696825344ae115c26ed03195a472383202607/src/components/SelectionList/BaseSelectionList.tsx#L465

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

M00rish commented 1 month ago

Proposal

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

Pressing space in split amount input goes to user's profile

What is the root cause of that problem?

when pressing space key while in split amount input (MoneyRequestAmountInput.tsx) onPress event is triggered, and then it propagates to the parent which is Pressable then the Onpress event gets fired there and so we go to the user's Profile

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

we can simply ignore the space key while we are in split amount input (MoneyRequestAmountInput.tsx), by adding this line : if (key === ' ' || key === 'spacebar) { nativeEvent.preventDefault(); }

to this function textInputKeyPress https://github.com/Expensify/App/blob/156b8b0da67d1df00b2dbc0aa0f25ed2f03f9ef3/src/components/MoneyRequestAmountInput.tsx#L245-L257

melvin-bot[bot] commented 1 month ago

📣 @M00rish! 📣 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>
Krishna2323 commented 1 month ago

Edited by proposal-police: This proposal was edited at 2024-09-07 04:48:59 UTC.

Proposal


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

Pressing space in the participant's split amount input goes to user's profile page.

What is the root cause of that problem?

The pressable element will role button will call onpress on space key press also. And we don't have a mechanism to prevent the onPress event of the PressableWithFeedback when spacebar is pressed on the input.

https://github.com/Expensify/App/blob/7814fbcf4d0da0d40050670d8129c8aaf759d67e/src/components/SelectionList/BaseListItem.tsx#L78-L87

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


OPTIONAL: We can call setMouseDown(); when input is focused and setMouseUp() when input is blurred.

What alternative solutions did you explore? (Optional)

We can change the role to menuitem

Result

https://github.com/user-attachments/assets/c0a894e4-018f-4850-aa03-267dca01e158

bondydaa commented 1 month ago

i'll be OOO next week so if we get proposals ready please ask for another engineer to review. cc @getusha

melvin-bot[bot] commented 1 month ago

@garrettmknight, @bondydaa, @getusha Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

getusha commented 1 month ago

Reviewing

getusha commented 1 month ago

@garrettmknight i don't think this is worth fixing, entering a space on a number input is unlikely to any user.

garrettmknight commented 1 month ago

I can get behind that.