-
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
Add a switch for requiring tags on the multi level tags RHP #42972
Merged
Merged
Changes from 40 commits
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
59c508b
feat: pass a new parameter to the requires tags api
6a2c8ee
chore: add new constant
336c036
feat: add the toggle button
5bcd53b
chore: disable the switch if all tags are not active
95c360b
fix: remove unnecessary import
75b21eb
fix: UI copy and error message
39f7fac
fix: component position
6a03be3
chore: remove unused error util
e1246a8
fix: swap the API to call
da8ca41
fix: add back the command to make all tags required
ec97f63
chore: add back the API param type
18e3d27
chore: add back the API command
ffd3507
chore: export the parameter type
21c49fe
fix: add back the old API call
b477d14
fix: remove extra parameters
3a450df
fix: remove null
276f6e3
fix: errors null location
53a9514
chore: remove redundant variable
abcfc1e
chore: remove zero
8b777f1
chore: use pendingFields
e2e37f2
chore: remove a const that is not used
55cd7bb
don't lock required switch when the switch is on
3c18dd6
chore: add generic error
7e5a4db
chore: add error fields
aa2127a
chore: clear pending fields
cdd4b49
fix: clear pending field
144ed26
fix: display error from the error field
fbbca9d
clear the error
fe47bb7
use action instead of fields
4a5851d
clear fields
1f083ef
clear the required field
9752216
fix: clear specific error field
86685bd
chore: define a function to clear a field error from policy tag list
b4422fb
refactor: get orderWeight instead of the tag name
4fc37fb
fix: the function parameter
7229be7
chore: make clearPolicyTagErrors take tag list index
70f7c6b
fix: use orderWeight from route params
1ca0837
chore: pass missing argument to the clear error function
327cc7d
Merge branch 'main' into hayata-add-requires-tags-button
83bca4b
fix: pass the missing argument to the function to clear the error
d6f5800
Merge branch 'main' into hayata-add-requires-tags-button
cdd4365
only clear policy tag error when the items renders are policy tags
050471f
fix: lint issue
bef9b95
return of policy tag name is undefined
9b97a25
Merge branch 'main' of github.com:Expensify/App into hayata-add-requi…
aldo-expensify File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
type SetPolicyTagsRequired = { | ||
policyID: string; | ||
tagListIndex: number; | ||
requireTagList: boolean; | ||
}; | ||
|
||
export default SetPolicyTagsRequired; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,7 @@ import * as PolicyUtils from '@libs/PolicyUtils'; | |
import type {SettingsNavigatorParamList} from '@navigation/types'; | ||
import NotFoundPage from '@pages/ErrorPage/NotFoundPage'; | ||
import AccessOrNotFoundWrapper from '@pages/workspace/AccessOrNotFoundWrapper'; | ||
import ToggleSettingOptionRow from '@pages/workspace/workflows/ToggleSettingsOptionRow'; | ||
import * as Tag from '@userActions/Policy/Tag'; | ||
import CONST from '@src/CONST'; | ||
import ONYXKEYS from '@src/ONYXKEYS'; | ||
|
@@ -65,10 +66,9 @@ function WorkspaceViewTagsPage({route}: WorkspaceViewTagsProps) { | |
setSelectedTags({}); | ||
}, [isFocused]); | ||
|
||
const policyTagList = useMemo(() => PolicyUtils.getTagLists(policyTags).find((policyTag) => policyTag.name === currentTagListName), [currentTagListName, policyTags]); | ||
const tagList = useMemo<TagListItem[]>( | ||
() => | ||
Object.values(policyTagList?.tags ?? {}) | ||
Object.values(currentPolicyTag?.tags ?? {}) | ||
.sort((tagA, tagB) => localeCompare(tagA.name, tagB.name)) | ||
.map((tag) => ({ | ||
value: tag.name, | ||
|
@@ -81,7 +81,7 @@ function WorkspaceViewTagsPage({route}: WorkspaceViewTagsProps) { | |
isDisabled: tag.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, | ||
rightElement: <ListItemRightCaretWithLabel labelText={tag.enabled ? translate('workspace.common.enabled') : translate('workspace.common.disabled')} />, | ||
})), | ||
[policyTagList, selectedTags, translate], | ||
[currentPolicyTag, selectedTags, translate], | ||
); | ||
|
||
const tagListKeyedByName = useMemo( | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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) |
||
title={translate('common.required')} | ||
switchAccessibilityLabel={translate('common.required')} | ||
isActive={Boolean(currentPolicyTag?.required)} | ||
onToggle={(on) => Tag.setPolicyTagsRequired(policyID, on, route.params.orderWeight)} | ||
pendingAction={currentPolicyTag.pendingFields?.required} | ||
errors={currentPolicyTag?.errorFields?.required ?? undefined} | ||
onCloseError={() => Tag.clearPolicyTagListError(policyID, route.params.orderWeight, 'required')} | ||
disabled={!currentPolicyTag?.required && !Object.values(currentPolicyTag?.tags ?? {}).some((tag) => tag.enabled)} | ||
/> | ||
</View> | ||
<OfflineWithFeedback | ||
errors={currentPolicyTag.errors} | ||
pendingAction={currentPolicyTag.pendingAction} | ||
|
@@ -264,7 +276,9 @@ function WorkspaceViewTagsPage({route}: WorkspaceViewTagsProps) { | |
customListHeader={getCustomListHeader()} | ||
shouldPreventDefaultFocusOnSelectRow={!DeviceCapabilities.canUseTouchScreen()} | ||
listHeaderWrapperStyle={[styles.ph9, styles.pv3, styles.pb5]} | ||
onDismissError={(item) => Tag.clearPolicyTagErrors(policyID, item.value)} | ||
onDismissError={(item) => { | ||
Tag.clearPolicyTagErrors(policyID, item.value, route.params.orderWeight); | ||
}} | ||
/> | ||
)} | ||
</ScreenWrapper> | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The line above makes it looks like policyTag could end up being
{}
, what happens then ifpolicyTag.name
isundefined
? I think we should avoid callingOnyx.merge
in such case.if
policyTag
is not expected to end up as{}
, then I think we shouldn't have?? {}
.