Skip to content

[C+ Checklist Needs Completion] [$1000] Keyboard navigation and selection is inconsistent in Split Bill UI #21392

Closed

Description

@lanitochka17

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


Issue found when executing PR #21310

Action Performed:

  1. Open http://staging.new.expensify.com/
  2. Open Group DM
  3. Click on "+" near message composer
  4. Click on "Split bill"
  5. Insert amount
  6. Click "Next"
  7. Click with mouse on any member to deselect it
  8. Use the keyboard navigation (up/down arrows + enter) to select/deselect different members

Expected Result:

User can successfully select/deselect members for split bill using keyboard navigation

Actual Result:

Keyboard navigation starts working incorrectly while selecting/deselecting members if user clicked on one of the members before. When user press "enter" for any member - select checkmark immediately returns to previous state. Sometimes 2 members are selected / deselected instantly instead of 1

Workaround:

Unknown

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.31.2

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Bug6103631_Recording__726.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~015b770640f03d0c88
  • Upwork Job ID: 1672370582030045184
  • Last Price Increase: 2023-11-08
  • Automatic offers:
    • mollfpr | Reviewer | 27885262
    • dukenv0307 | Contributor | 27885264

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Jun 23, 2023
  2. melvin-bot commented on Jun 23, 2023

    @melvin-bot

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

  3. melvin-bot commented on Jun 23, 2023

    @melvin-bot

    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
  4. greg-schroeder commented on Jun 23, 2023

    @greg-schroeder
    Contributor

    I reproduced this. Although the keyboard navigation seems fine to me, the "enter" functionality does seem to be buggy:

    2023-06-23_15-24-52.mp4
  5. melvin-bot commented on Jun 23, 2023

    @melvin-bot

    Looks like something related to react-navigation may have been mentioned in this issue discussion.

    As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

    Feel free to drop a note in #expensify-open-source with any questions.

  6. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Jun 23, 2023
  7. changed the title [-]Split bill - Keyboard works incorrectly selecting members if click on them before[/-] [+][$1000] Split bill - Keyboard works incorrectly selecting members if click on them before[/+] on Jun 23, 2023
  8. melvin-bot commented on Jun 23, 2023

    @melvin-bot
  9. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Jun 23, 2023
  10. melvin-bot commented on Jun 23, 2023

    @melvin-bot

    Current assignee @greg-schroeder is eligible for the External assigner, not assigning anyone new.

  11. melvin-bot commented on Jun 23, 2023

    @melvin-bot

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

  12. changed the title [-][$1000] Split bill - Keyboard works incorrectly selecting members if click on them before[/-] [+][$1000] Keyboard navigation and selection is inconsistent in Split Bill UI[/+] on Jun 23, 2023
  13. 155 remaining items

  14. greg-schroeder commented on Jan 24, 2024

    @greg-schroeder
    Contributor

    Okay - can you confirm for me ASAP @mollfpr? I'm going to hold off on the payment for now until I know what the payment amount should be taking regressions into account

  15. mollfpr commented on Jan 25, 2024

    @mollfpr
    Contributor

    I tested the changes on the PR #32464, and yes, it did cause the issue here: #34761.

    The selected proposal and the root cause explanation indicate that we wrongly passed the component as a function, which makes the issue.

    @greg-schroeder I'm not sure how we seeing this as a regression or a side effect of the improvement we did.

  16. greg-schroeder commented on Jan 25, 2024

    @greg-schroeder
    Contributor

    @mollfpr Yeah I see what you're saying. While it was maybe caused by it, it's not really a regression in the traditional sense

  17. greg-schroeder commented on Jan 25, 2024

    @greg-schroeder
    Contributor

    The upwork job for this closed a long time ago due to inactivity so I will create a new one

  18. greg-schroeder commented on Jan 25, 2024

    @greg-schroeder
    Contributor

    Offers sent to @mollfpr and @dukenv0307

    Can you complete the checklist @mollfpr? Thanks!

  19. changed the title [-][HOLD for payment 2024-01-24] [$1000] Keyboard navigation and selection is inconsistent in Split Bill UI[/-] [+][C+ Checklist Needs Completion] [$1000] Keyboard navigation and selection is inconsistent in Split Bill UI[/+] on Jan 25, 2024
  20. mollfpr commented on Jan 26, 2024

    @mollfpr
    Contributor

     [@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
     @mollfpr] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

    No offending PR.

    [@mollfpr] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

    The regression step should be good.

    [@mollfpr] Determine if we should create a regression test for this bug.
    [@mollfpr] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again

    1. Create a group chat
    2. Click on the plus icon > split bill
    3. Enter the amount to go to confirm page
    4. Click on the checkbox of one member
    5. Use the arrow key to focus on another
    6. Click enter key
    7. Verify that the member which has the checkbox is focused is selected/de-selected
    8. 👍 or 👎
  21. mollfpr commented on Jan 26, 2024

    @mollfpr
    Contributor

    @greg-schroeder You can cancel the offer for me; I'll do a manual request in NewDot. Could you give me the payment summary? Thank you!

  22. greg-schroeder commented on Jan 26, 2024

    @greg-schroeder
    Contributor

    Oh, woops, I forgot, I'll cancel the offer.

    @mollfpr is due $1k for this job

  23. greg-schroeder commented on Jan 26, 2024

    @greg-schroeder
    Contributor

    Filed regression test, closing this out, I'll pay you right away once you accept offer @dukenv0307

  24. JmillsExpensify commented on Jan 26, 2024

    @JmillsExpensify
    Contributor

    $1,000 approved for @mollfpr based on this comment.

  25. dukenv0307 commented on Jan 29, 2024

    @dukenv0307
    Contributor

    Filed regression test, closing this out, I'll pay you right away once you accept offer @dukenv0307

    @greg-schroeder Accepted, thank you!

  26. melvin-bot commented on Feb 13, 2024

    @melvin-bot

    ⚠️ Looks like this issue was linked to a Deploy Blocker here

    If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

    If a regression has occurred and you are the assigned CM follow the instructions here.

    If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

BugSomething is broken. Auto assigns a BugZero manager.ExternalAdded to denote the issue can be worked on by a contributorWeeklyKSv2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions