-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Implement same tag splitting logic we have in the API #36978
Conversation
2788f05
to
ade9319
Compare
ade9319
to
13583b5
Compare
@yuwenmemon we may also want to use |
Ah yes thank you @Ollyws - updated! |
Reviewer Checklist
Screenshots/Videos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@marcochavezf Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, all yours @AndrewGable
Thanks! We don't need to wait on Andrew |
Implement same tag splitting logic we have in the API (cherry picked from commit 4a885f5)
CP-d to staging and should be testable in version 1.4.43-18. |
🚀 Deployed to production by https://github.com/puneetlath in version: 1.4.43-20 🚀
|
@AndrewGable please review
cc @rezkiy37
Details
@rezkiy37 My bad I should have thought about this in the review but splitting tags ends up not becoming as simple as splitting by the colon delimiter. I'm just copying some logic from JS in OldDot since it's tried and true. A good follow-up will be to move this to expensify-common so it can be used by both repos but I don't know how much we want to put a transaction util method there or if it's worth the hassle as we move over to NewDot.
Fixed Issues
$ #36916
Tests/QA
Preconditions:
In OldDot under admin, create a Collect group policy, enable tags and add the tag "Child: Tom", add the employee to the policy. (For Applause, setup OldDot Collect Policy instructions here:
https://sites.google.com/applausemail.com/applause-expensifyproject/wiki-guides/newdot-categories?authuser=0)
Do the same steps as above but with a policy that has multiple levels of tags enabled. Make sure you can still select all tags.
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop