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 2024-01-11] [$500] Update all selectors to use new format for selected participant #29916

Closed
thienlnam opened this issue Oct 18, 2023 · 41 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@thienlnam
Copy link
Contributor

thienlnam commented Oct 18, 2023

Follow up from #29903

In #29836, we updated the flow for the group chat / split bill flow to hide the selected participants when we are searching for a user. We'll need to update the flow for all selectors to use that same format

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b5750587ffb7e229
  • Upwork Job ID: 1714778343126532096
  • Last Price Increase: 2023-11-08
  • Automatic offers:
    • abdulrahuman5196 | Reviewer | 27740728
    • dukenv0307 | Contributor | 27740730
@thienlnam thienlnam added the NewFeature Something to build that is a new item. label Oct 18, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2023

@thienlnam
Copy link
Contributor Author

Waiting to add external until the linked PR is merged

@thienlnam thienlnam added the External Added to denote the issue can be worked on by a contributor label Oct 18, 2023
@melvin-bot melvin-bot bot changed the title Update all selectors to use new format for selected participant [$500] Update all selectors to use new format for selected participant Oct 18, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 18, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2023

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Oct 18, 2023
@thienlnam
Copy link
Contributor Author

We're still looking for proposals in the standard format

@dukenv0307
Copy link
Contributor

dukenv0307 commented Oct 19, 2023

Proposal

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

We'll need to update the flow for all selectors to use that same format for selected participant

What is the root cause of that problem?

We recently updated the selected participant format in #29836 and we need to follow that same format in other places.

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

  1. Look for the OptionsSelector that has canSelectMultipleOptions property passed in, and SelectionList that has canSelectMultiple passed in. Since those will be affected by this UX change.

There're some that use canSelectMultiple that already work fine (does not use getOptions) like in WorkspaceMembersPage, we can evaluate them during the PR process.
2. Pass the includeSelectedOptions as true in the getOptions that gets the options for those places, similar to here
3. Only push the participants section if search term is empty, similar to here

Note: we have a current bug with the flow here that's being fixed separately, whatever solution being selected there will be applied to the above steps as well.

What alternative solutions did you explore? (Optional)

NA

@melvin-bot

This comment was marked as resolved.

@mallenexpensify
Copy link
Contributor

@abdulrahuman5196, can review @dukenv0307 's proposal above plz?

@dukenv0307
Copy link
Contributor

dukenv0307 commented Oct 20, 2023

@abdulrahuman5196 FYI @thienlnam seems 👍 on me handling this issue as per this comment

@thienlnam
Copy link
Contributor Author

Yup, please review the proposal and work with Duke to update if there are any issues with it and then we can move to PR

@abdulrahuman5196
Copy link
Contributor

@dukenv0307 I think still we expect a formatted proposal as always even though you would get dibs here. We should still add some missing pieces in the proposal like below.

There're some that use canSelectMultiple that already work fine (does not use getOptions) like in WorkspaceMembersPage, we can evaluate them during the PR process.

We should provide information on this. This is not a straightforward forward change in my opinion.
Like do we make it to migrate to getOptions or like update existing functions in WorkspaceMembersPage to behave like the new selection mechanism?

Pass the includeSelectedOptions as true in the getOptions that gets the options for those places, similar to here

  1. Only push the participants section if search term is empty, similar to here

And we should also provide information on what pages/code are these which needs to be updated?

@melvin-bot melvin-bot bot added the Overdue label Oct 23, 2023
@abdulrahuman5196
Copy link
Contributor

Melvin we had just posted update before weekend and back from weekend

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 23, 2023
@mallenexpensify
Copy link
Contributor

@dukenv0307 can you please address @abdulrahuman5196 's comments in their post above?

@melvin-bot melvin-bot bot removed the Overdue label Oct 25, 2023
@melvin-bot melvin-bot bot added the Overdue label Nov 19, 2023
Copy link

melvin-bot bot commented Nov 19, 2023

📣 @abdulrahuman5196 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Nov 19, 2023

📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@abdulrahuman5196
Copy link
Contributor

Not overdue melvin, we just assigned the contributor

@melvin-bot melvin-bot bot removed the Overdue label Nov 20, 2023
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Nov 22, 2023
@dukenv0307
Copy link
Contributor

@abdulrahuman5196 The PR is ready for review

@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Dec 15, 2023
Copy link

melvin-bot bot commented Dec 15, 2023

This issue has not been updated in over 15 days. @mallenexpensify, @thienlnam, @abdulrahuman5196, @dukenv0307 eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@thienlnam thienlnam added Weekly KSv2 and removed Monthly KSv2 labels Dec 15, 2023
@thienlnam
Copy link
Contributor Author

What's the update here @abdulrahuman5196?

@abdulrahuman5196
Copy link
Contributor

Hi, Will be reviewing the PR again today.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 4, 2024
@melvin-bot melvin-bot bot changed the title [$500] Update all selectors to use new format for selected participant [HOLD for payment 2024-01-11] [$500] Update all selectors to use new format for selected participant Jan 4, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jan 4, 2024
Copy link

melvin-bot bot commented Jan 4, 2024

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

Copy link

melvin-bot bot commented Jan 4, 2024

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

If no regressions arise, payment will be issued on 2024-01-11. 🎊

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.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Jan 4, 2024

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

  • [@abdulrahuman5196] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
  • [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@abdulrahuman5196
Copy link
Contributor

  1. Go to a room/WS and open the Memember page
  2. Click Invite and choose some users
  3. Verify that you see them at the top
  4. Start searching for a user and verify only the participants matching the selection is shown in the list, whereas the selected participants matching the search is shown at top and the rest matching the search is followed
  5. When there is no search, all the selected participants should show at the top and the rest following them.

@mallenexpensify

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jan 11, 2024
@mallenexpensify
Copy link
Contributor

Contributor: @dukenv0307 paid $500 via Upwork
Contributor+: @abdulrahuman5196 paid $500 via Upwork

TestRail update steps GH created https://github.com/Expensify/Expensify/issues/359909
Thanks @abdulrahuman5196

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
None yet
Development

No branches or pull requests

5 participants