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.54k stars 2.89k forks source link

[WAITING ON SLACK THREAD] [$500] mWeb - Holding down on the "No thanks" or "Download" buttons on the banner selects previous words #27101

Closed kbecciv closed 1 year ago

kbecciv commented 1 year 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!


Action Performed:

  1. Sign in for the first time on a new account on mWeb
  2. When a banner shows up press and hold down on either "No thanks" or "Download" buttons

Expected Result:

No words should be selected

Actual Result:

The "." gets selected on the paragraph

Workaround:

Unknown

Platforms:

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

Version Number: 1.3.66.3 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 Notes/Photos/Videos: Any additional supporting documentation

https://github.com/Expensify/App/assets/93399543/c8e5c55c-165b-4ff1-8815-a07996a4bac1

https://github.com/Expensify/App/assets/93399543/9c3560e3-cac4-48e2-a26e-54de98320e99

Expensify/Expensify Issue URL: Issue reported by: @Nathan-Mulugeta Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693906785297509

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~013927694479c35846
  • Upwork Job ID: 1700787122331496448
  • Last Price Increase: 2023-09-10
melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

ishpaul777 commented 1 year ago

Proposal

Problem

Holding down on the "No thanks" or "Download" buttons on the banner selects previous words

Root cause

We are not using css style user-select: none for disabling the selection on buttion

Solution

If change is specific to Popover buttons like "Download" and "No thanks" buttons. Use user-select: none for disabling the selection on "Cancel" and "OK" buttons in ConfirmContent

Add here style={[styles.mt4, styles.userSelectNone]}

Add here style={[styles.mt3, styles.noSelect, styles.userSelectNone]}

Add here style={[styles.noSelect, styles.flex1, styles.userSelectNone]}

Add here style={[styles.flex1, styles.userSelectNone]}

Alternatively, we can disable userSelectNone to global Button Component and and enable userSelect selectively by passing userSelectText to styles prop, wherever requires.

ZhenjaHorbach commented 1 year ago

Proposal

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

Holding the buttons on the DownloadApp modal selects the previous text

What is the root cause of that problem?

Since the text in the buttons is not selectable

https://github.com/Expensify/App/blob/b11bddc054f54bb46d5fb5aaf05e25b8a7df7faf/src/components/Button/index.js#L208

Instead, the button component selects the previous text

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

Since the text in the buttons is not selectable It would be nice to do the same with the buttons itself

And add styles.userSelectNone for these styles

https://github.com/Expensify/App/blob/b11bddc054f54bb46d5fb5aaf05e25b8a7df7faf/src/components/Button/index.js#L288-L300

https://github.com/Expensify/App/assets/68128028/45aba9ae-6901-49c4-84d5-8b66984cf56e

What alternative solutions did you explore? (Optional)

NA

jliexpensify commented 1 year ago

Unassigning Kev, I'm Bug Zero. Not sure why 2 of us are assigned.

jliexpensify commented 1 year ago

@kbecciv were you told to add the External label, or is this a new process? Please note that this assigns the bug to Upworks - this is something that the Bug Zero team (i.e. me) would typically do. Some of my colleagues have also noticed you adding this label on other GH's.

jliexpensify commented 1 year ago

Left my thoughts here, but I'm not convinced this is a bug we'd fix - the reasoning behind this is that when faced with 2 buttons, users wouldn't be long-pressing. So while this is an inconvenience if the buttons were long-pressed, I don't think it will affect users due to the fact they wouldn't be doing this action.

kbecciv commented 1 year ago

Hey @jliexpensify QA team have new instructions for the Contributor's bug. Here is the link in qa channel https://expensify.slack.com/archives/C9YU7BX5M/p1694149069725469

jliexpensify commented 1 year ago

Thanks @kbecciv - we weren't told of this update until yesterday, and there were other Applause GH's without the label applied, so some confusion there from our side!