Add a switch for requiring tags on the multi level tags RHP #42972
Conversation
|
@ 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] |
|
The required label and location is not aligned with the design from #41279 
|
|
Update: For updating the requires tags in the account level, we have to use another API. |
|
@s77rt, so the API for requiring tags in the context of multi level tags is different from requiring tagging for all tags in the workspace. Add the necessary code to call this different API command |
Co-authored-by: Abdelhafidh Belalia <16493223+s77rt@users.noreply.github.com>
|
@hayata-suenaga Still need one more ts cbeck |
|
@s77rt, this is ready for another review 🙇 |
Co-authored-by: Abdelhafidh Belalia <16493223+s77rt@users.noreply.github.com>
|
@hayata-suenaga lint please |
| @@ -359,10 +360,21 @@ function clearPolicyTagErrors(policyID: string, tagName: string) { | |||
| }); | |||
| } | |||
|
|
|||
| function clearPolicyTagListError(policyID: string, tagListIndex: number, errorField: string) { | |||
| const policyTag = PolicyUtils.getTagLists(allPolicyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`] ?? {})?.[tagListIndex] ?? {}; | |||
|
|
|||
There was a problem hiding this comment.
| if (!policyTag.name) { | |
| return; | |
| } |
The line above makes it looks like policyTag could end up being {}, what happens then if policyTag.name is undefined? I think we should avoid calling Onyx.merge in such case.
if policyTag is not expected to end up as {}, then I think we shouldn't have ?? {}.
|
Seems like we have some lint issues |
|
@Gonals I pushed main and the styles seems to not be complaining now. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
Thank you for merging, Aldo! 🙇 |
|
🚀 Deployed to production by https://github.com/mountiny in version: 1.4.83-3 🚀
|
| @@ -234,6 +234,18 @@ function WorkspaceViewTagsPage({route}: WorkspaceViewTagsProps) { | |||
| cancelText={translate('common.cancel')} | |||
| danger | |||
| /> | |||
| <View style={[styles.pv4, styles.ph5]}> | |||
| <ToggleSettingOptionRow | |||
There was a problem hiding this comment.
Coming from this issue #43758, we should have hide the required option for dependent tags, because dependent tag lists cannot be required. More context #43758 (comment) #43758 (comment)
Details
This PR adds a switch component on the RHP that appears when the user clicks a multi-level tag (only available when tags are imported from an accounting connection).
With this switch, the admin can decide whether to require tagging on each category (classes or customers/projects).
Fixed Issues
$ #41374
PROPOSAL: N/A
Tests / QA Steps
Offline tests
N/A
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2024-06-02.at.2.30.12.PM.mov
MacOS: Desktop