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

[$250] Update the Group Creation flow #54460

Open
mountiny opened this issue Dec 23, 2024 · 25 comments
Open

[$250] Update the Group Creation flow #54460

mountiny opened this issue Dec 23, 2024 · 25 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@mountiny
Copy link
Contributor

mountiny commented Dec 23, 2024

Problem

Coming from this discussion, let's update the group chat creation flow to remove the ambiguity. Right now, users have to keep clicking exactly to the Add to Group button to keep adding people to the group, but if they misclick out of the button to the row, the DM is opened and the progress on group creation is lost.

Solution

Let's update the flow such that:

  • Once the first group member is selected with the Add to Group button, the button remains, but clicking the entire row selects or deselects the user for the group (the circle with checkmark indicates the user is selected for the group)
  • If no user is selected, clicking the row opens/ creates a DM as it does now
  • if at least one user is selected for the group chat, you cannot create a DM
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021872417451020324244
  • Upwork Job ID: 1872417451020324244
  • Last Price Increase: 2025-01-02
  • Automatic offers:
    • FitseTLT | Contributor | 105595571
Issue OwnerCurrent Issue Owner: @hungvu193
@mountiny mountiny added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 23, 2024
@mountiny mountiny self-assigned this Dec 23, 2024
Copy link

melvin-bot bot commented Dec 23, 2024

Triggered auto assignment to @VictoriaExpensify (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.

@mountiny mountiny moved this to MEDIUM in [#whatsnext] #quality Dec 23, 2024
@mkzie2
Copy link
Contributor

mkzie2 commented Dec 23, 2024

Edited by proposal-police: This proposal was edited at 2024-12-27 04:17:17 UTC.

Proposal

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

Update the Group Creation flow

What is the root cause of that problem?

Currently, if we click on the row, we will always navigate to the DM chat

const createChat = useCallback(
(option?: OptionsListUtils.Option) => {
if (option?.isSelfDM) {
Navigation.dismissModal(option.reportID);

We only display the check mark if the row is selected

if (item.isSelected) {

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

  1. When we click on a row, if selectedOptions is an empty array, navigate to the DM chat. If not we will deselect/select this options. I think for the selfDM, we will always navigate to the selfDM report if we select this row.
const onSelectRow = useCallback(
    (option?: OptionsListUtils.Option) => {
        if (option?.isSelfDM) {
            Navigation.dismissModal(option.reportID);
            return;
        }
        if (selectedOptions.length > 0) {
            const isOptionInList = !!option?.isSelected;

            let newSelectedOptions;

            if (isOptionInList) {
                newSelectedOptions = reject(selectedOptions, (selectedOption) => selectedOption.login === option.login);
            } else {
                newSelectedOptions = [...selectedOptions, {...option, isSelected: true, selected: true, reportID: option?.reportID ?? '-1'}];
            }

            selectionListRef?.current?.clearInputAfterSelect?.();

            setSelectedOptions(newSelectedOptions);
        } else {
            createChat(option);
        }
    },
    [createChat, selectedOptions, setSelectedOptions],
);

onSelectRow={onSelectRow}

const createChat = useCallback(
(option?: OptionsListUtils.Option) => {
if (option?.isSelfDM) {
Navigation.dismissModal(option.reportID);

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

@mountiny
Copy link
Contributor Author

This is not open to the proposals yet

@melvin-bot melvin-bot bot added the Overdue label Dec 26, 2024
@mountiny
Copy link
Contributor Author

Updated the OP

@melvin-bot melvin-bot bot removed the Overdue label Dec 26, 2024
@mountiny mountiny added External Added to denote the issue can be worked on by a contributor Overdue labels Dec 26, 2024
Copy link

melvin-bot bot commented Dec 26, 2024

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

@melvin-bot melvin-bot bot changed the title Update the Group Creation flow [$250] Update the Group Creation flow Dec 26, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 26, 2024
Copy link

melvin-bot bot commented Dec 26, 2024

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

@mkzie2
Copy link
Contributor

mkzie2 commented Dec 27, 2024

@mountiny That means we will display both Add to group button and check mark when the user is selected, right?

@mkzie2
Copy link
Contributor

mkzie2 commented Dec 27, 2024

Updated proposal

@hungvu193
Copy link
Contributor

Will wait for confirmation before reviewing proposals.

@mountiny
Copy link
Contributor Author

No, when the user is selected the add to group button wont show as they already selected, we will only show the checkmark circle on such row.

@mkzie2
Copy link
Contributor

mkzie2 commented Dec 27, 2024

@mountiny Then if this user is de-selected we will show the Add to group button, right? Or we will display the circle without a checkmark.

@FitseTLT
Copy link
Contributor

FitseTLT commented Dec 27, 2024

Proposal

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

Update the Group Creation flow

What is the root cause of that problem?

Feature Improvement

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

We will just take the toggleOption here above createChat
and add a line of code to toggleOption and early return if there are selected options in createChat

if (selectedOptions.length) {
                toggleOption(option);
                return;
            }

Another problem we will have is what would happen if the user clicks on selfDM when there are selected participants

  1. open the self DM: no code change needed
  2. Don't display selfDM in the list when when there are selected participants (this is more logical), filter out selfDM
    data: recentReports,
            data: selectedOptions.length ? recentReports.filter((option) => !option.isSelfDM) : recentReports,

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

@mountiny
Copy link
Contributor Author

@mkzie2 we would show the button again.

@hungvu193 can you please review the proposals now? Thanks!

@hungvu193
Copy link
Contributor

@hungvu193 can you please review the proposals now? Thanks!

Sure

@hungvu193
Copy link
Contributor

@mountiny I wanna clarify this requirement here:

if at least one user is selected for the group chat, you cannot create a DM

Does that mean when at least one member is selected, clicking the entire row will add that user to the selected options?

@mkzie2
Copy link
Contributor

mkzie2 commented Dec 29, 2024

Updated proposal. Just removed the point 2 since it's working as expected now.

@hungvu193
Copy link
Contributor

Still waiting for the confirmation

Copy link

melvin-bot bot commented Jan 2, 2025

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@hungvu193
Copy link
Contributor

@FitseTLT 's proposal here LGTM, it requires less changes and can reuse existing logic.

🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented Jan 6, 2025

Current assignee @mountiny is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@mkzie2
Copy link
Contributor

mkzie2 commented Jan 6, 2025

if (selectedOptions.length > 0) {
const isOptionInList = !!option?.isSelected;

        let newSelectedOptions;

        if (isOptionInList) {
            newSelectedOptions = reject(selectedOptions, (selectedOption) => selectedOption.login === option.login);
        } else {
            newSelectedOptions = [...selectedOptions, {...option, isSelected: true, selected: true, reportID: option?.reportID ?? '-1'}];
        }

        selectionListRef?.current?.clearInputAfterSelect?.();

        setSelectedOptions(newSelectedOptions);
    }

@hungvu193 This logic is what I copied from toggleOption and it's easy to reuse it in the PR phrase if we want to do this.

Copy link

melvin-bot bot commented Jan 6, 2025

@hungvu193 @mountiny @VictoriaExpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@hungvu193
Copy link
Contributor

@hungvu193 This logic is what I copied from toggleOption and it's easy to reuse it in the PR phrase if we want to do this.

Hey @mkzie2 I'd choose the most completed proposal, even when I consider accepting your opinion, your updated proposal was still after @FitseTLT posted his proposal.

All yours @mountiny

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 7, 2025
Copy link

melvin-bot bot commented Jan 7, 2025

📣 @FitseTLT 🎉 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 📖

@mountiny
Copy link
Contributor Author

mountiny commented Jan 7, 2025

Thanks!

@FitseTLT what is your eta for the pr?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
Development

No branches or pull requests

5 participants