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.56k stars 2.9k forks source link

[HOLD for payment 2023-07-17] [$1000] Web - Mouse cursor is shown as hand icon (on a not clickable section) on who paid, split bill #21695

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. Go to '+" -> new group -> select members
  2. Click create group
  3. Now it should show a new group page
  4. Go to ''+" on new group page -> split bill -> type the amount -> next
  5. Notice who paid? card section, the cursor is shown as the mouse pointer

Expected Result:

The arrow cursor should be shown instead of mouse cursor as the who paid section ( where the user who paid is shown)

Actual Result:

Mouse cursor is shown as hand icon (on a not clickable section) on who paid, split bill

Workaround:

Unknown

Platforms:

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

Version Number: 1.3.30-5 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/f7b61150-3c9e-404e-a6ae-8cde0a2d9f59

https://github.com/Expensify/App/assets/93399543/181ad4d6-6f06-4ee2-899d-4b90304bca1b

Expensify/Expensify Issue URL: Issue reported by: @ashimsharma10 Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1687527115940319

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~013377045cdf090308
  • Upwork Job ID: 1673779406484242432
  • Last Price Increase: 2023-06-27
melvin-bot[bot] commented 1 year ago

Triggered auto assignment to @lschurr (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)

ginsuma commented 1 year ago

Proposal

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

Web - Mouse cursor is shown as hand icon (on a not clickable section) on who paid, split

What is the root cause of that problem?

What we expected:

Split Bill From Group https://github.com/Expensify/App/assets/13113013/b8aaacc6-9cca-4977-be0f-231db65c0db3
Split Bill From FAB https://github.com/Expensify/App/assets/13113013/f0eeb825-b0fa-445e-8f23-e1ce2d0f4d37

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

We add isReadOnly prop to OptionRow component. This prop is used for 'Who paid' section when can modify participants.

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Current assignee @lschurr is eligible for the External assigner, not assigning anyone new.

melvin-bot[bot] commented 1 year ago

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

Santhosh-Sellavel commented 1 year ago

@ginsuma Your proposal didn't work for me, please check again!

ginsuma commented 1 year ago

@Santhosh-Sellavel Screen Shot 2023-06-28 at 5 26 54 AM My proposal shows like that. Can you describe how should it show?

Santhosh-Sellavel commented 1 year ago

Sorry for the confusion, my apologies it works. Something wrong with my setup. Working now.

Santhosh-Sellavel commented 1 year ago

@ginsuma's Proposal looks good here.

For CME This issue raises minor concerns,

On the confirmation page, while we create a SplitBill from a group chat, we will be able to unselect users from Who was there? but in the flow where we create a SpiltBill from the FAB button, we will not be able to unselect anyone still we allow to click on all the users and will show the User Details page. I believe post this proposal we will not be able to click Who paid in FAB flow also, are we fine with that, or should exclude that?

Recording Before on staging & now on dev

https://github.com/Expensify/App/assets/85645967/c9984146-8aec-46c8-98bd-0c00c5c799d7

C+ Reviewed πŸŽ€ πŸ‘€ πŸŽ€

cc: @lschurr

melvin-bot[bot] commented 1 year ago

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

ginsuma commented 1 year ago

@Santhosh-Sellavel For your concern, we can edit it to for working both cases:

const canModifyParticipants = !props.isReadOnly && props.canModifyParticipants && props.hasMultipleParticipants;
 {
    title: translate('moneyRequestConfirmationList.whoPaid'),
    data: [formattedPayeeOption],
    shouldShow: true,
    indexOffset: 0,
    isDisabled: canModifyParticipants,
},
ginsuma commented 1 year ago

Proposal

Updated

Santhosh-Sellavel commented 1 year ago

Cool @ginsuma, I'm not sure whether we want that, thanks anyway! Let's wait for @amyevans!

amyevans commented 1 year ago

I'm not sure that disabling the Who Paid section is the way we should go from a design perspective (for instance, I could see an alternative being removing the circle/checkmark altogether). So I'd like @Expensify/design's eyes on it first before we proceed.

Santhosh-Sellavel commented 1 year ago

Good call @amyevans!

shawnborton commented 1 year ago

I like keeping the checkmark to indicate that the person is included, but I think I would just make it so you can't interact with it.

ginsuma commented 1 year ago

Can you check the below is what we expected:

Split Bill From Group https://github.com/Expensify/App/assets/13113013/b8aaacc6-9cca-4977-be0f-231db65c0db3
Split Bill From FAB https://github.com/Expensify/App/assets/13113013/f0eeb825-b0fa-445e-8f23-e1ce2d0f4d37
Santhosh-Sellavel commented 1 year ago

@shawnborton Can you confirm this works for you?

shawnborton commented 1 year ago

Works for me!

amyevans commented 1 year ago

Cool, assigning @ginsuma!

melvin-bot[bot] commented 1 year ago

πŸ“£ @Santhosh-Sellavel You have been assigned to this job! Please apply to this job in Upwork here and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing πŸ“–

melvin-bot[bot] commented 1 year ago

πŸ“£ @ginsuma You have been assigned to this job! Please apply to this job in Upwork here and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing πŸ“–

melvin-bot[bot] commented 1 year ago

πŸ“£ @ashimsharma10 You have been assigned to this job! Please apply to this job in Upwork here and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing πŸ“–

ashimsharma10 commented 1 year ago

Bot is not assigning properly. Please someone check.

ginsuma commented 1 year ago

https://github.com/Expensify/App/pull/21945 PR is up @amyevans @Santhosh-Sellavel .

amyevans commented 1 year ago

Heads up I'm OOO Mon/Tue next week.

melvin-bot[bot] commented 1 year ago

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

On to the next one πŸš€

melvin-bot[bot] commented 1 year ago

@amyevans, @ginsuma, @lschurr, @Santhosh-Sellavel Huh... This is 4 days overdue. Who can take care of this?

Santhosh-Sellavel commented 1 year ago

PR is Merged!

melvin-bot[bot] commented 1 year ago

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot[bot] commented 1 year ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.3.38-7 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-07-17. :confetti_ball:

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

melvin-bot[bot] commented 1 year ago

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

lschurr commented 1 year ago

@ashimsharma10 @Santhosh-Sellavel @ginsuma - please apply for the job in Upwork here: https://www.upwork.com/jobs/~013377045cdf090308

Santhosh-Sellavel commented 1 year ago

@lschurr Will collect on ND.

ashimsharma10 commented 1 year ago

@lschurr applied!

ginsuma commented 1 year ago

I applied. Thanks.

lschurr commented 1 year ago

@Santhosh-Sellavel could you work on the checklist? Do we need a regression test?

Santhosh-Sellavel commented 1 year ago

I think we can skip the checklist here as it's not a regression but rather a minor UX polish. cc: @amyevans thoughts?

amyevans commented 1 year ago

Agreed!

lschurr commented 1 year ago

Thanks all! I'll issue payments on Monday and close.

lschurr commented 1 year ago

@ashimsharma10 - could you clarify what's going on in Upwork? Did you accept the offer?

ashimsharma10 commented 1 year ago

@lschurr it is shown "withdrawn offer". Can u please offer me again at https://www.upwork.com/freelancers/~018a92cf13e1e88eed.

lschurr commented 1 year ago

Payments are sent. Closing.

Santhosh-Sellavel commented 1 year ago

Requested for payment on ND. ($1000)

anmurali commented 1 year ago

Approved based on https://github.com/Expensify/App/issues/21695#issuecomment-1636173729