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

[Wave 6: Tags][$500][HOLD https://github.com/Expensify/App/issues/32735] Tag - Two tags are highlighted when one sub-tag is selected #32521

Closed
3 of 6 tasks
lanitochka17 opened this issue Dec 5, 2023 · 41 comments
Assignees
Labels
Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Dec 5, 2023

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


Version Number: Y
Reproducible in staging?: N
Reproducible in production?:
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

Precondition: User is a member of workspace that has some tags in Parent: Child format. There has to be at least 9 tags in the policy.

  1. Go to the workspace chat
  2. Click + > Request money > Manual
  3. Enter amount > Next
  4. Click Show more > Tag
  5. Select a subcategory
  6. Click Tag

Expected Result:

Only the selected tag is highlighted

Actual Result:

Two tags are highlighted when a sub-tag is selected

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

Add any screenshot/video evidence

Bug6302305_1701805501243.20231206_004546.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01da048e9d51f0937c
  • Upwork Job ID: 1732341978058440704
  • Last Price Increase: 2023-12-13
@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Dec 5, 2023
Copy link
Contributor

github-actions bot commented Dec 5, 2023

👋 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.

Copy link

melvin-bot bot commented Dec 5, 2023

Triggered auto assignment to @aldo-expensify (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@aldo-expensify
Copy link
Contributor

aldo-expensify commented Dec 5, 2023

@lanitochka17 what was the account used to reproduce this? Otherwise, can we have clearer steps on how to create the tags in the "Precondition"? I tried creating a workspace with multilevel tags in OldDot and I don't see what you see in the video. I don't see multiple levels.

@aldo-expensify
Copy link
Contributor

I'm only seeing one level in NewDot:

image

I setup up "dependent" multilevel tags in OldDot.

@lanitochka17
Copy link
Author

lanitochka17 commented Dec 5, 2023

@aldo-expensify Hello
Follow the instructions in the file, but as for tags:
https://expensify.testrail.io/index.php?/runs/view/19265&group_by=cases:section_id&group_order=asc&group_id=296764

image

image

@aldo-expensify
Copy link
Contributor

Thanks, I got the tags working now!

@aldo-expensify
Copy link
Contributor

I'm failing to reproduce this in staging:

  1. I select a nested tag
  2. I click again the tag to open the selection menu
  3. The only highlighted tag is the selected one
Screen.Recording.2023-12-05.at.1.35.02.PM.mov

@aldo-expensify
Copy link
Contributor

@lanitochka17 what is the account email used in the test?

@aldo-expensify
Copy link
Contributor

Removing deploy blocker since I can't reproduce and doesn't look very important to hold the deploy on it

@aldo-expensify aldo-expensify added Daily KSv2 Needs Reproduction Reproducible steps needed and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Dec 5, 2023
@lanitochka17
Copy link
Author

@aldo-expensify tags must be more than 9

@aldo-expensify
Copy link
Contributor

I'll try that

@aldo-expensify
Copy link
Contributor

@lanitochka17 thanks, I managed to reproduce now:

image

Considering that nested tags is not handled in production yet (it is a new feature), I think it is fine we don't block the deploy on it and we fix improve it as a follow up

@lanitochka17
Copy link
Author

@aldo-expensify thanks

@aldo-expensify aldo-expensify added the Bug Something is broken. Auto assigns a BugZero manager. label Dec 6, 2023
Copy link

melvin-bot bot commented Dec 6, 2023

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

Copy link

melvin-bot bot commented Dec 6, 2023

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

@dylanexpensify dylanexpensify added the External Added to denote the issue can be worked on by a contributor label Dec 6, 2023
@dukenv0307
Copy link
Contributor

dukenv0307 commented Dec 6, 2023

Proposal

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

Two tags are highlighted when a sub-tag is selected

What is the root cause of that problem?

We're not setting isOneLine true in getTagsOptions of selected options here similar to what we're doing with the category selected options. So the selected tags will show as nested rather than "Parent: Child" which is the correct pattern. It also causes this issue.

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

Set isOneLine true in getTagsOptions of selected options here

We need to pass it through from getTagsOptions to getIndentedOptionTree here as well

What alternative solutions did you explore? (Optional)

We can consider doing the same for the search options, this will make it consistent with the categories

@melvin-bot melvin-bot bot added the Overdue label Dec 8, 2023
@amyevans amyevans changed the title [$500] Tag - Two tags are highlighted when one sub-tag is selected [Wave 6: Tags][$500] Tag - Two tags are highlighted when one sub-tag is selected Dec 8, 2023
@aldo-expensify
Copy link
Contributor

@Santhosh-Sellavel thoughts on the proposal: #32521 (comment) ?

@melvin-bot melvin-bot bot removed the Overdue label Dec 8, 2023
@Santhosh-Sellavel
Copy link
Collaborator

I haven't tested it out yet, the proposal sounds good to me.

Can I get a CSV file for multilevel tags?

@dukenv0307
Copy link
Contributor

dukenv0307 commented Dec 11, 2023

@Santhosh-Sellavel FYI we can test it by setting the Tags to Onyx (note to replace with the correct policy id). The Accounting below is the one with children.

Onyx.set('policyTags_${YOUR-POLICY-ID}', {
    Department: {
        name: 'Department',
        tags: {
            'Accounting: Child 1': {
                name: 'Accounting: Child 1',
                enabled: true,
            },
            'Accounting: Child 2': {
                name: 'Accounting: Child 2',
                enabled: true,
            },
            Design: {
                name: 'Design',
                enabled: true,
            },
            Engineering: {
                name: 'Engineering',
                enabled: true,
            },
            HR: {
                name: 'HR',
                enabled: true,
            },
            Infra: {
                name: 'Infra',
                enabled: true,
            },
            Legal: {
                name: 'Legal',
                enabled: true,
            },
            Marketing: {
                name: 'Marketing',
                enabled: true,
            },
            Ops: {
                name: 'Ops',
                enabled: true,
            },
            Sales: {
                name: 'Sales',
                enabled: true,
            },
        },
    },
});

@dukenv0307
Copy link
Contributor

@Santhosh-Sellavel After the fix, it will correctly look like this (similar to the Categories)

Screenshot 2023-12-11 at 2 12 57 PM

@dylanexpensify
Copy link
Contributor

@Santhosh-Sellavel to review!

@aldo-expensify
Copy link
Contributor

aldo-expensify commented Dec 12, 2023

Can I get a CSV file for multilevel tags?

@Santhosh-Sellavel you don't need the "Do you want to use multiple levels of" option, you have to create normal tags using the format parent: child. These instructions are for categories, but they also apply for tags and I reproduced using them.

Note: you also need to create at least 9 tags

@Santhosh-Sellavel
Copy link
Collaborator

Can I get a CSV file for multilevel tags?

@Santhosh-Sellavel you don't need the "Do you want to use multiple levels of" option, you have to create normal tags using the format parent: child. These instructions are for categories, but they also apply for tags and I reproduced using them.

Note: you also need to create at least 9 tags

For Categories worked fine, but for Tags it didn't work for me. Will try again though.

@Santhosh-Sellavel
Copy link
Collaborator

Santhosh-Sellavel commented Dec 12, 2023

@aldo-expensify Are you taking over this as CME, lets go with @dukenv0307 proposal here

@aldo-expensify
Copy link
Contributor

oh, so the expected results in the GH description is wrong? Tags are not expected to behave as categories when in form Parent: child? I understand that @dukenv0307 's proposal will disable the nesting.

@dylanexpensify can you help me confirm what is the expected behaviour here? Should tags in form Parent: child show nested? or is that only for categories?

@dukenv0307
Copy link
Contributor

oh, so the expected results in the GH description is wrong? Tags are not expected to behave as categories when in form Parent: child? I understand that @dukenv0307 's #32521 (comment) will disable the nesting.

@aldo-expensify Tags in form of Parent: child will still show nested in the tag list (like the Accounting > Child 2 below), my proposal is only so that the selected one will not show nested, so it will look like this (which is consistent with the categories)
Screenshot 2023-12-13 at 1 58 24 PM

Currently it looks quite bad, like this which are 2 nested tag groups that are disconnected from each other, it's not clear if the first Accounting is the list section header or the tag group parent (also it's inconsistent with categories UI)
Screenshot 2023-12-13 at 2 00 52 PM

cc @dylanexpensify

@dylanexpensify
Copy link
Contributor

Hmmm @puneetlath @greg-schroeder any ideas?

@aldo-expensify
Copy link
Contributor

@aldo-expensify Tags in form of Parent: child will still show nested in the tag list (like the Accounting > Child 2 below), my proposal is only so that the selected one will not show nested, so it will look like this (which is consistent with the categories)

Oh, I understand now, for a moment I thought it was going to remove all nesting, but it only does for the selected item. That looks good to me then, specially if it is consistent with categories.

Copy link

melvin-bot bot commented Dec 13, 2023

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

@Santhosh-Sellavel
Copy link
Collaborator

@aldo-expensify Let's assign @dukenv0307 here, thanks!

@aldo-expensify
Copy link
Contributor

@dylanexpensify I'll proceed with hiring @dukenv0307 since I think the path is clear enough

@puneetlath
Copy link
Contributor

Tags are not supposed to be nested. That was implemented by mistake and is being reverted.

@aldo-expensify
Copy link
Contributor

ohh, so the expectation is wrong. Do we have an issue where the revert is being handled? If we do, we should HOLD on that or just close this.

@aldo-expensify
Copy link
Contributor

aldo-expensify commented Dec 13, 2023

Sorry @dukenv0307 for the confusion, I'll remove you while we figure out the way forward.

Ignore me.. I see I never assigned you anyway :P

@puneetlath
Copy link
Contributor

Yep! #32735

@aldo-expensify aldo-expensify changed the title [Wave 6: Tags][$500] Tag - Two tags are highlighted when one sub-tag is selected [Wave 6: Tags][$500][HOLD https://github.com/Expensify/App/issues/32735] Tag - Two tags are highlighted when one sub-tag is selected Dec 13, 2023
@aldo-expensify aldo-expensify added Weekly KSv2 and removed Daily KSv2 labels Dec 13, 2023
@aldo-expensify
Copy link
Contributor

Thanks @puneetlath !

Added HOLD so we can check once #32735 is implemented, most likely this won't be an issue anymore.

@aldo-expensify aldo-expensify removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 13, 2023
@greg-schroeder
Copy link
Contributor

Closing this following the revert

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Weekly KSv2
Projects
No open projects
Development

No branches or pull requests

7 participants