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] Categories - Check mark appears briefly next to "Enabled" after tapping Select #46651

Closed
2 of 6 tasks
izarutskaya opened this issue Aug 1, 2024 · 21 comments
Closed
2 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@izarutskaya
Copy link

izarutskaya commented Aug 1, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 9.0.15-4
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4801321
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

  1. Launch New Expensify app.
  2. Go to workspace settings > Categories.
  3. Long press on any category.
  4. Tap Select.

Expected Result:

When long pressing on the category, there will be no check mark next to "Enabled".

Actual Result:

When long pressing on the category, a green check mark appears briefly next to "Enabled".

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6558895_1722493005603!Screenshot_2024-08-01_141301

Bug6558895_1722493005519.ScreenRecording_08-01-2024_14-09-47_1__1_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~014c73364e46974b4a
  • Upwork Job ID: 1818974271362784186
  • Last Price Increase: 2024-08-01
  • Automatic offers:
    • alitoshmatov | Reviewer | 103436909
Issue OwnerCurrent Issue Owner: @alitoshmatov
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. DeployBlocker Indicates it should block deploying the API labels Aug 1, 2024
Copy link

melvin-bot bot commented Aug 1, 2024

Triggered auto assignment to @cristipaval (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

Copy link

melvin-bot bot commented Aug 1, 2024

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

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Aug 1, 2024
Copy link
Contributor

github-actions bot commented Aug 1, 2024

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@izarutskaya
Copy link
Author

We think this issue might be related to the #collect project.

@cristipaval cristipaval added External Added to denote the issue can be worked on by a contributor Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API Hourly KSv2 labels Aug 1, 2024
@melvin-bot melvin-bot bot changed the title Categories - Check mark appears briefly next to "Enabled" after tapping Select [$250] Categories - Check mark appears briefly next to "Enabled" after tapping Select Aug 1, 2024
Copy link

melvin-bot bot commented Aug 1, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 1, 2024
Copy link

melvin-bot bot commented Aug 1, 2024

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

@cristipaval
Copy link
Contributor

Not a blocker, demoting.

@bernhardoj
Copy link
Contributor

Proposal

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

Check mark appears briefly when selecting item in list.

What is the root cause of that problem?

The checkmark will show if the item is selected and canSelectMultiple is false

{!canSelectMultiple && item.isSelected && !rightHandSideComponent && (
<View
style={[styles.flexRow, styles.alignItemsCenter, styles.ml3]}
accessible={false}
>
<View>
<Icon
src={Expensicons.Checkmark}
fill={theme.success}
/>
</View>
</View>
)}

canSelectMultiple is only true if the selection mode is on.

const canSelectMultiple = shouldUseNarrowLayout ? selectionMode?.isEnabled : true;

However, the selection mode toggling waits for the onyx data while the category item is already selected.

const turnOnSelectionMode = () => {
turnOnMobileSelectionMode();

const turnOnMobileSelectionMode = () => {
Onyx.merge(ONYXKEYS.MOBILE_SELECTION_MODE, {isEnabled: true});
};

So, item.isSelected is already true, but canSelectMultiple is still false.

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

Update the isSelected condition so it's true only if canSelectMultiple is true.

isSelected: !!selectedCategories[value.name],

isSelected: !!selectedCategories[value.name] && canSelectMultiple,

Copy link

melvin-bot bot commented Aug 6, 2024

@cristipaval, @mallenexpensify, @alitoshmatov Huh... This is 4 days overdue. Who can take care of this?

@melvin-bot melvin-bot bot added the Overdue label Aug 6, 2024
@cristipaval
Copy link
Contributor

@alitoshmatov Could you please review the proposals?

@alitoshmatov
Copy link
Contributor

Thank you for your proposal @bernhardoj . Your RCA is correct. But your solution is not working properly on me.

Notice when first time selection is enabled it is not showing selected item, and only updating after second item is selected

Simulator.Screen.Recording.-.iPhone.15.Plus.-.2024-08-07.at.16.03.46.mp4

Also we should consider all similar cases throughout the app. Like tags page also have the same issue

@melvin-bot melvin-bot bot removed the Overdue label Aug 7, 2024
@bernhardoj
Copy link
Contributor

@alitoshmatov did you add canSelectMultiple to the categoryList deps?

@alitoshmatov
Copy link
Contributor

@bernhardoj My bad, That was the problem. It is working fine now.

@alitoshmatov
Copy link
Contributor

We can go with @bernhardoj 's proposal.

We should also fix tags page and other similar pages if any exists.

C+ reviewed 🎀 👀 🎀

Copy link

melvin-bot bot commented Aug 7, 2024

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

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

melvin-bot bot commented Aug 7, 2024

📣 @alitoshmatov 🎉 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

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Aug 8, 2024
@bernhardoj
Copy link
Contributor

PR is ready

cc: @alitoshmatov

@bernhardoj
Copy link
Contributor

The PR was deployed 2 weeks ago, so this should be ready for payment.

Requested in ND.

@JmillsExpensify
Copy link

Need a payment summary to approve.

@mallenexpensify mallenexpensify added Daily KSv2 and removed Reviewing Has a PR in review Weekly KSv2 labels Aug 30, 2024
@mallenexpensify
Copy link
Contributor

Contributor: @bernhardoj due $250 via NewDot
Contributor+: @alitoshmatov paid $250 via Upwork.

I'm leaning towards - we don't need a regression test for this since it's a brief flash and not something we want to check on with every deploy (and questionable if we'd want to check monthly). Comment if you disagree.

@JmillsExpensify
Copy link

$250 approved for @bernhardoj

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 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

6 participants