From f75446aef0820e46801305f9e5f4cb2c949707db Mon Sep 17 00:00:00 2001 From: "truph01 (via MelvinBot)" Date: Sat, 22 Aug 2026 03:56:28 +0000 Subject: [PATCH 1/6] Gate workspace picker selection behind a Save button (a11y On Input, PR 11) Co-authored-by: truph01 --- .../DynamicReportChangeWorkspacePage.tsx | 20 ++++++++--- src/pages/SetDefaultWorkspacePage.tsx | 20 ++++++++--- .../DomainGroupPreferredWorkspacePage.tsx | 34 +++++++++++++------ 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/src/pages/DynamicReportChangeWorkspacePage.tsx b/src/pages/DynamicReportChangeWorkspacePage.tsx index bbf7c62d8ff6..66188bba3304 100644 --- a/src/pages/DynamicReportChangeWorkspacePage.tsx +++ b/src/pages/DynamicReportChangeWorkspacePage.tsx @@ -54,7 +54,7 @@ import type {DismissedProductTraining} from '@src/types/onyx'; import type {OnyxEntry} from 'react-native-onyx'; import {isTrackIntentUserSelector} from '@selectors/Onboarding'; -import React from 'react'; +import React, {useState} from 'react'; import {View} from 'react-native'; import type {WithReportOrNotFoundProps} from './inbox/report/withReportOrNotFound'; @@ -117,6 +117,9 @@ function DynamicReportChangeWorkspacePage({report}: DynamicReportChangeWorkspace const {currentSearchResults} = useSearchResultsContext(); const shouldCalculateTotals = useSearchShouldCalculateTotals(currentSearchKey, currentSearchQueryJSON?.hash, true); + const [draftPolicyID, setDraftPolicyID] = useState(); + const currentSelection = draftPolicyID ?? report.policyID; + // The snapshot keeps the report row after a workspace change, and only the server can tell whether it still matches the query. const refreshSearch = () => { refreshSearchAfterReportAction({ @@ -215,7 +218,7 @@ function DynamicReportChangeWorkspacePage({report}: DynamicReportChangeWorkspace policies, currentUserLogin: session?.email, shouldShowPendingDeletePolicy: false, - selectedPolicyIDs: report.policyID ? [report.policyID] : undefined, + selectedPolicyIDs: currentSelection ? [currentSelection] : undefined, searchTerm: debouncedSearchTerm, localeCompare, additionalFilter: (newPolicy) => { @@ -235,6 +238,13 @@ function DynamicReportChangeWorkspacePage({report}: DynamicReportChangeWorkspace headerMessage: shouldShowNoResultsFoundMessage ? translate('common.noResultsFound') : '', }; + const confirmButtonOptions = { + showButton: true, + text: translate('common.save'), + onConfirm: () => selectPolicy(currentSelection), + isDisabled: !currentSelection || currentSelection === report.policyID, + }; + if (!isMoneyRequestReport(report) || isMoneyRequestReportPendingDeletion(report) || hasCommuterExclusionDistanceRequest) { return ; } @@ -242,7 +252,7 @@ function DynamicReportChangeWorkspacePage({report}: DynamicReportChangeWorkspace return ( {({didScreenTransitionEnd}) => ( @@ -261,11 +271,13 @@ function DynamicReportChangeWorkspacePage({report}: DynamicReportChangeWorkspace ListItem={UserListItem} data={data} - onSelectRow={(option) => selectPolicy(option.policyID)} + onSelectRow={(option) => setDraftPolicyID(option.policyID)} textInputOptions={textInputOptions} + confirmButtonOptions={confirmButtonOptions} initiallyFocusedItemKey={report.policyID} shouldShowLoadingPlaceholder={fetchStatus.status === 'loading' || !didScreenTransitionEnd} disableMaintainingScrollPosition + addBottomSafeAreaPadding /> )} diff --git a/src/pages/SetDefaultWorkspacePage.tsx b/src/pages/SetDefaultWorkspacePage.tsx index 91cfc0b3ca65..97f8e052b338 100644 --- a/src/pages/SetDefaultWorkspacePage.tsx +++ b/src/pages/SetDefaultWorkspacePage.tsx @@ -27,7 +27,7 @@ import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type SCREENS from '@src/SCREENS'; -import React, {useMemo} from 'react'; +import React, {useMemo, useState} from 'react'; import {View} from 'react-native'; type SetDefaultWorkspacePageProps = PlatformStackScreenProps; @@ -46,6 +46,9 @@ function SetDefaultWorkspacePage({route}: SetDefaultWorkspacePageProps) { const shouldShowLoadingIndicator = isAppLoadPending && !isOffline; const session = useSession(); + const [draftPolicyID, setDraftPolicyID] = useState(); + const currentSelection = draftPolicyID ?? activePolicyID; + const selectPolicy = (selectedPolicyID?: string) => { if (!selectedPolicyID) { return; @@ -72,12 +75,19 @@ function SetDefaultWorkspacePage({route}: SetDefaultWorkspacePageProps) { policies, currentUserLogin: session?.email, shouldShowPendingDeletePolicy: false, - selectedPolicyIDs: undefined, + selectedPolicyIDs: draftPolicyID ? [draftPolicyID] : undefined, searchTerm: debouncedSearchTerm, localeCompare, additionalFilter: (newPolicy) => isGroupPolicy(newPolicy), }); + const confirmButtonOptions = { + showButton: true, + text: translate('common.save'), + onConfirm: () => selectPolicy(currentSelection), + isDisabled: currentSelection === activePolicyID, + }; + const textInputOptions = useMemo( () => ({ label: shouldShowSearchInput ? translate('common.search') : undefined, @@ -91,7 +101,7 @@ function SetDefaultWorkspacePage({route}: SetDefaultWorkspacePageProps) { return ( {({didScreenTransitionEnd}) => ( @@ -109,9 +119,11 @@ function SetDefaultWorkspacePage({route}: SetDefaultWorkspacePageProps) { data={data} ListItem={UserListItem} textInputOptions={textInputOptions} - onSelectRow={(option) => selectPolicy(option.policyID)} + onSelectRow={(option) => setDraftPolicyID(option.policyID)} + confirmButtonOptions={confirmButtonOptions} shouldShowLoadingPlaceholder={fetchStatus.status === 'loading' || !didScreenTransitionEnd} disableMaintainingScrollPosition + addBottomSafeAreaPadding /> )} diff --git a/src/pages/domain/Groups/DomainGroupPreferredWorkspacePage.tsx b/src/pages/domain/Groups/DomainGroupPreferredWorkspacePage.tsx index 33e9d7190905..36297133b230 100644 --- a/src/pages/domain/Groups/DomainGroupPreferredWorkspacePage.tsx +++ b/src/pages/domain/Groups/DomainGroupPreferredWorkspacePage.tsx @@ -23,7 +23,7 @@ import type SCREENS from '@src/SCREENS'; import {domainSecurityGroupSettingPendingActionSelector, selectGroupByID} from '@selectors/Domain'; import {createAdminPoliciesSelector} from '@selectors/Policy'; -import React from 'react'; +import React, {useState} from 'react'; type WorkspaceListItem = { policyID: string; @@ -48,6 +48,9 @@ function DomainGroupPreferredWorkspacePage({route}: DomainGroupPreferredWorkspac const currentPolicyID = group?.restrictedPrimaryPolicyID; + const [selectedPolicyID, setSelectedPolicyID] = useState(); + const currentSelection = selectedPolicyID ?? currentPolicyID; + const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: createAdminPoliciesSelector(currentPolicyID)}); const workspaceOptions: WorkspaceListItem[] = []; @@ -61,10 +64,25 @@ function DomainGroupPreferredWorkspacePage({route}: DomainGroupPreferredWorkspac policyID: policy.id, created: policy.created, keyForList: policy.id, - isSelected: currentPolicyID === policy.id, + isSelected: currentSelection === policy.id, }); } + const saveWorkspace = () => { + if (!group || !currentSelection) { + return; + } + updateDomainSecurityGroup(domainAccountID, groupID, group, {restrictedPrimaryPolicyID: currentSelection}, 'restrictedPrimaryPolicyID'); + Navigation.goBack(ROUTES.DOMAIN_GROUP_DETAILS.getRoute(domainAccountID, groupID)); + }; + + const confirmButtonOptions = { + showButton: true, + text: translate('common.save'), + onConfirm: saveWorkspace, + isDisabled: currentSelection === currentPolicyID, + }; + return ( data={workspaceOptions.sort((a, b) => localeCompare(a.created ?? '', b.created ?? ''))} ListItem={UserListItem} - onSelectRow={(item: WorkspaceListItem) => { - if (!group) { - return; - } - updateDomainSecurityGroup(domainAccountID, groupID, group, {restrictedPrimaryPolicyID: item.policyID}, 'restrictedPrimaryPolicyID'); - Navigation.goBack(ROUTES.DOMAIN_GROUP_DETAILS.getRoute(domainAccountID, groupID)); - }} + onSelectRow={(item: WorkspaceListItem) => setSelectedPolicyID(item.policyID)} + confirmButtonOptions={confirmButtonOptions} initiallyFocusedItemKey={currentPolicyID} shouldUpdateFocusedIndex + addBottomSafeAreaPadding /> From 2d89ed73a1ef1791ffb706f428ade7b45e6605c0 Mon Sep 17 00:00:00 2001 From: "truph01 (via MelvinBot)" Date: Wed, 9 Sep 2026 11:06:24 +0000 Subject: [PATCH 2/6] Keep the saved workspace pinned to the top instead of the unsaved draft selection Co-authored-by: truph01 --- src/hooks/useWorkspaceList.ts | 16 ++++++++++++++-- src/pages/DynamicReportChangeWorkspacePage.tsx | 2 ++ src/pages/SetDefaultWorkspacePage.tsx | 2 ++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/hooks/useWorkspaceList.ts b/src/hooks/useWorkspaceList.ts index e8c78a78a9c8..f6705abbbea2 100644 --- a/src/hooks/useWorkspaceList.ts +++ b/src/hooks/useWorkspaceList.ts @@ -25,6 +25,12 @@ type UseWorkspaceListParams = { localeCompare: LocaleContextProps['localeCompare']; additionalFilter?: (policy: OnyxEntry) => boolean; shouldSortSelectedToTop?: boolean; + + /** + * Policy IDs to pin to the top of the list, defaulting to `selectedPolicyIDs`. Pass the saved value when the list checks an unsaved + * draft selection, so checking a row highlights it without reordering the list under the user. + */ + policyIDsToSortToTop?: string[]; includeArchivedPolicy?: boolean; }; @@ -37,6 +43,7 @@ function useWorkspaceList({ localeCompare, additionalFilter, shouldSortSelectedToTop = true, + policyIDsToSortToTop, includeArchivedPolicy = false, }: UseWorkspaceListParams) { const icons = useMemoizedLazyExpensifyIcons(['FallbackWorkspaceAvatar']); @@ -81,11 +88,16 @@ function useWorkspaceList({ () => tokenizedSearch(usersWorkspaces, searchTerm, (policy) => [policy.text]).sort((policy1, policy2) => { if (shouldSortSelectedToTop) { - return sortWorkspacesBySelected({policyID: policy1.policyID, name: policy1.text}, {policyID: policy2.policyID, name: policy2.text}, selectedPolicyIDs, localeCompare); + return sortWorkspacesBySelected( + {policyID: policy1.policyID, name: policy1.text}, + {policyID: policy2.policyID, name: policy2.text}, + policyIDsToSortToTop ?? selectedPolicyIDs, + localeCompare, + ); } return localeCompare(policy1.text, policy2.text); }), - [searchTerm, usersWorkspaces, selectedPolicyIDs, localeCompare, shouldSortSelectedToTop], + [searchTerm, usersWorkspaces, selectedPolicyIDs, policyIDsToSortToTop, localeCompare, shouldSortSelectedToTop], ); const sections = useMemo(() => { diff --git a/src/pages/DynamicReportChangeWorkspacePage.tsx b/src/pages/DynamicReportChangeWorkspacePage.tsx index ead698f2fe54..343c64c736c5 100644 --- a/src/pages/DynamicReportChangeWorkspacePage.tsx +++ b/src/pages/DynamicReportChangeWorkspacePage.tsx @@ -221,6 +221,8 @@ function DynamicReportChangeWorkspacePage({report}: DynamicReportChangeWorkspace currentUserLogin: session?.email, shouldShowPendingDeletePolicy: false, selectedPolicyIDs: currentSelection ? [currentSelection] : undefined, + // The report's saved workspace stays pinned to the top, so checking another row doesn't reorder the list before the user saves. + policyIDsToSortToTop: report.policyID ? [report.policyID] : undefined, searchTerm: debouncedSearchTerm, localeCompare, additionalFilter: (newPolicy) => { diff --git a/src/pages/SetDefaultWorkspacePage.tsx b/src/pages/SetDefaultWorkspacePage.tsx index 97f8e052b338..8036aaf33b95 100644 --- a/src/pages/SetDefaultWorkspacePage.tsx +++ b/src/pages/SetDefaultWorkspacePage.tsx @@ -76,6 +76,8 @@ function SetDefaultWorkspacePage({route}: SetDefaultWorkspacePageProps) { currentUserLogin: session?.email, shouldShowPendingDeletePolicy: false, selectedPolicyIDs: draftPolicyID ? [draftPolicyID] : undefined, + // This page never pinned a workspace to the top, so keep the plain alphabetical order instead of pinning the row the user just checked. + shouldSortSelectedToTop: false, searchTerm: debouncedSearchTerm, localeCompare, additionalFilter: (newPolicy) => isGroupPolicy(newPolicy), From a6c0d0863619e451188e9e1e319d8a5c355f2b8f Mon Sep 17 00:00:00 2001 From: "truph01 (via MelvinBot)" Date: Wed, 9 Sep 2026 20:54:11 +0000 Subject: [PATCH 3/6] Move the Save gate into BaseDomainGroupPreferredWorkspacePage The draft state, the checkmark and the Save button are one concern, so owning them in the shared component instead of threading draftPolicyID and confirmButtonOptions through each caller. Callers now pass a single shouldConfirmSelection flag, and onSelectWorkspace means the same thing in both modes: commit this workspace. This also makes the broken prop combinations unrepresentable - there is no longer a way to pass a draft with no Save button, or a Save button with no draft. --- .../BaseDomainGroupPreferredWorkspacePage.tsx | 40 +++++++++++++------ .../DomainGroupPreferredWorkspacePage.tsx | 34 +++++----------- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/src/pages/domain/Groups/BaseDomainGroupPreferredWorkspacePage.tsx b/src/pages/domain/Groups/BaseDomainGroupPreferredWorkspacePage.tsx index 35a350f577fc..27541b6f9ff9 100644 --- a/src/pages/domain/Groups/BaseDomainGroupPreferredWorkspacePage.tsx +++ b/src/pages/domain/Groups/BaseDomainGroupPreferredWorkspacePage.tsx @@ -1,14 +1,15 @@ /** * Shared preferred-workspace selector for domain groups. Both the group-create and group-edit * pages delegate to this component; it renders the admin workspace list with a search field and - * gates access behind DomainNotFoundPageWrapper. + * gates access behind DomainNotFoundPageWrapper. Callers choose how a pick commits: immediately on + * row press, or staged until Save via `shouldConfirmSelection`. */ import type {FullPageNotFoundViewProps} from '@components/BlockingViews/FullPageNotFoundView'; import HeaderWithBackButton from '@components/HeaderWithBackButton'; import ScreenWrapper from '@components/ScreenWrapper'; import SelectionList from '@components/SelectionList'; import UserListItem from '@components/SelectionList/ListItem/UserListItem'; -import type {ConfirmButtonOptions, ListItem} from '@components/SelectionList/types'; +import type {ListItem} from '@components/SelectionList/types'; import Text from '@components/Text'; import useLocalize from '@hooks/useLocalize'; @@ -24,7 +25,7 @@ import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import {createAdminPoliciesSelector} from '@selectors/Policy'; -import React from 'react'; +import React, {useState} from 'react'; type WorkspaceListItem = { policyID: string; @@ -40,14 +41,14 @@ type BaseDomainGroupPreferredWorkspacePageProps = { /** The policy ID of the saved preferred workspace. It stays in the list even when the user is not its admin, and it is the row focused on open. */ selectedPolicyID: string | undefined; - /** The policy ID to check instead of `selectedPolicyID`, for pages that stage the pick and only commit it on Save */ - draftPolicyID?: string; - - /** Called with the policy ID of the workspace the user picked */ + /** Commits the picked workspace. Fired on row press, or on Save when `shouldConfirmSelection` is set. */ onSelectWorkspace: (policyID: string) => void; - /** Options for the footer confirm button. Omit it on pages that commit the pick as soon as a row is selected. */ - confirmButtonOptions?: ConfirmButtonOptions; + /** + * Whether picking a row only stages the choice, leaving a Save button to commit it. Set it wherever committing on + * press would change the layout or navigate away, which WCAG 3.2.2 "On Input" disallows without an explicit confirm. + */ + shouldConfirmSelection?: boolean; /** Called when the back button is pressed */ onBackButtonPress: () => void; @@ -65,9 +66,8 @@ type BaseDomainGroupPreferredWorkspacePageProps = { function BaseDomainGroupPreferredWorkspacePage({ domainAccountID, selectedPolicyID, - draftPolicyID, onSelectWorkspace, - confirmButtonOptions, + shouldConfirmSelection = false, onBackButtonPress, testID, shouldBeBlocked, @@ -76,6 +76,8 @@ function BaseDomainGroupPreferredWorkspacePage({ const styles = useThemeStyles(); const {translate, localeCompare} = useLocalize(); + const [draftPolicyID, setDraftPolicyID] = useState(); + // The saved workspace drives which rows the list contains and which one opens focused, so staging a pick can't drop it or move the list under the user. const checkedPolicyID = draftPolicyID ?? selectedPolicyID; @@ -105,6 +107,20 @@ function BaseDomainGroupPreferredWorkspacePage({ // The search input is gated on the unfiltered list length so it doesn't disappear once a query narrows the results. const shouldShowSearchInput = workspaceOptions.length >= CONST.STANDARD_LIST_ITEM_LIMIT; + const confirmButtonOptions = shouldConfirmSelection + ? { + showButton: true, + text: translate('common.save'), + onConfirm: () => { + if (!checkedPolicyID) { + return; + } + onSelectWorkspace(checkedPolicyID); + }, + isDisabled: checkedPolicyID === selectedPolicyID, + } + : undefined; + return ( 0 && filteredWorkspaceOptions.length === 0 ? translate('common.noResultsFound') : '', }} - onSelectRow={(item: WorkspaceListItem) => onSelectWorkspace(item.policyID)} + onSelectRow={(item: WorkspaceListItem) => (shouldConfirmSelection ? setDraftPolicyID(item.policyID) : onSelectWorkspace(item.policyID))} confirmButtonOptions={confirmButtonOptions} initiallyFocusedItemKey={selectedPolicyID} shouldUpdateFocusedIndex diff --git a/src/pages/domain/Groups/DomainGroupPreferredWorkspacePage.tsx b/src/pages/domain/Groups/DomainGroupPreferredWorkspacePage.tsx index b853f211d710..5518f1c931df 100644 --- a/src/pages/domain/Groups/DomainGroupPreferredWorkspacePage.tsx +++ b/src/pages/domain/Groups/DomainGroupPreferredWorkspacePage.tsx @@ -1,4 +1,3 @@ -import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; import Navigation from '@navigation/Navigation'; @@ -12,7 +11,7 @@ import ROUTES from '@src/ROUTES'; import type SCREENS from '@src/SCREENS'; import {domainSecurityGroupSettingPendingActionSelector, selectGroupByID} from '@selectors/Domain'; -import React, {useState} from 'react'; +import React from 'react'; import BaseDomainGroupPreferredWorkspacePage from './BaseDomainGroupPreferredWorkspacePage'; @@ -21,8 +20,6 @@ type DomainGroupPreferredWorkspacePageProps = PlatformStackScreenProps(); - const currentSelection = draftPolicyID ?? currentPolicyID; - - const saveWorkspace = () => { - if (!group || !currentSelection) { - return; - } - updateDomainSecurityGroup(domainAccountID, groupID, group, {restrictedPrimaryPolicyID: currentSelection}, 'restrictedPrimaryPolicyID'); - Navigation.goBack(ROUTES.DOMAIN_GROUP_DETAILS.getRoute(domainAccountID, groupID)); - }; - return ( Navigation.goBack(ROUTES.DOMAIN_GROUPS.getRoute(domainAccountID)), }} onBackButtonPress={() => Navigation.goBack(ROUTES.DOMAIN_GROUP_DETAILS.getRoute(domainAccountID, groupID))} - onSelectWorkspace={setDraftPolicyID} - confirmButtonOptions={{ - showButton: true, - text: translate('common.save'), - onConfirm: saveWorkspace, - isDisabled: currentSelection === currentPolicyID, + shouldConfirmSelection + onSelectWorkspace={(policyID: string) => { + if (!group) { + return; + } + updateDomainSecurityGroup(domainAccountID, groupID, group, {restrictedPrimaryPolicyID: policyID}, 'restrictedPrimaryPolicyID'); + Navigation.goBack(ROUTES.DOMAIN_GROUP_DETAILS.getRoute(domainAccountID, groupID)); }} /> ); From 80cc7fb06287349a7c0300672b0e7560fc9e1341 Mon Sep 17 00:00:00 2001 From: "truph01 (via MelvinBot)" Date: Thu, 10 Sep 2026 02:37:42 +0000 Subject: [PATCH 4/6] Enable Save on SetDefaultWorkspacePage whenever a row is checked selectPolicy on this page also advances the flow - it navigates to navigateTo when the picked policy has categories enabled - so confirming the already-active workspace is a real action, unlike on the other two pickers where it would be a no-op. isDisabled compared currentSelection against activePolicyID while the checkmark was driven by draftPolicyID, so the two could disagree: tapping the active workspace checked the row but left Save dead, with no way to proceed. Gate on draftPolicyID instead, which is exactly the row the list shows as checked. Guarding on !currentSelection would instead enable Save on open whenever activePolicyID is set but no row is checked - reachable, since the page opens with a set activePolicyID when that policy is personal or missing - and confirming would write that policy back as the default. --- src/pages/SetDefaultWorkspacePage.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/pages/SetDefaultWorkspacePage.tsx b/src/pages/SetDefaultWorkspacePage.tsx index 8036aaf33b95..9c20295dc2ba 100644 --- a/src/pages/SetDefaultWorkspacePage.tsx +++ b/src/pages/SetDefaultWorkspacePage.tsx @@ -47,7 +47,6 @@ function SetDefaultWorkspacePage({route}: SetDefaultWorkspacePageProps) { const session = useSession(); const [draftPolicyID, setDraftPolicyID] = useState(); - const currentSelection = draftPolicyID ?? activePolicyID; const selectPolicy = (selectedPolicyID?: string) => { if (!selectedPolicyID) { @@ -86,8 +85,10 @@ function SetDefaultWorkspacePage({route}: SetDefaultWorkspacePageProps) { const confirmButtonOptions = { showButton: true, text: translate('common.save'), - onConfirm: () => selectPolicy(currentSelection), - isDisabled: currentSelection === activePolicyID, + onConfirm: () => selectPolicy(draftPolicyID), + // Unlike the other two pickers, selectPolicy also advances the flow (it navigates to navigateTo), so confirming the + // already-active workspace is a real action. Gate on whether a row is checked, not on whether the value changed. + isDisabled: !draftPolicyID, }; const textInputOptions = useMemo( From 92376e66957205946ce84ed2806408af55f93e53 Mon Sep 17 00:00:00 2001 From: "truph01 (via MelvinBot)" Date: Thu, 10 Sep 2026 02:48:57 +0000 Subject: [PATCH 5/6] Remove redundant comments Drops the comments that restated the code they sat on, and trims the rest back to the one-line style of their surrounding files: - useWorkspaceList: drop the policyIDsToSortToTop JSDoc block. No other param in that type is documented, and the name plus the 'policyIDsToSortToTop ?? selectedPolicyIDs' fallback already say it. - DynamicReportChangeWorkspacePage: drop the policyIDsToSortToTop comment, which restated the prop name and its value. - BaseDomainGroupPreferredWorkspacePage: drop the file-header sentence that duplicated the shouldConfirmSelection prop doc, drop the checkedPolicyID comment (it described lines other than its own), and restore selectedPolicyID/onSelectWorkspace to their original wording. - SetDefaultWorkspacePage: keep both comments but cut the halves that restated the flag they annotate. Comment-only change; no logic touched. --- src/hooks/useWorkspaceList.ts | 5 ----- src/pages/DynamicReportChangeWorkspacePage.tsx | 1 - src/pages/SetDefaultWorkspacePage.tsx | 5 ++--- .../BaseDomainGroupPreferredWorkspacePage.tsx | 14 ++++---------- 4 files changed, 6 insertions(+), 19 deletions(-) diff --git a/src/hooks/useWorkspaceList.ts b/src/hooks/useWorkspaceList.ts index f6705abbbea2..3de8e86dc005 100644 --- a/src/hooks/useWorkspaceList.ts +++ b/src/hooks/useWorkspaceList.ts @@ -25,11 +25,6 @@ type UseWorkspaceListParams = { localeCompare: LocaleContextProps['localeCompare']; additionalFilter?: (policy: OnyxEntry) => boolean; shouldSortSelectedToTop?: boolean; - - /** - * Policy IDs to pin to the top of the list, defaulting to `selectedPolicyIDs`. Pass the saved value when the list checks an unsaved - * draft selection, so checking a row highlights it without reordering the list under the user. - */ policyIDsToSortToTop?: string[]; includeArchivedPolicy?: boolean; }; diff --git a/src/pages/DynamicReportChangeWorkspacePage.tsx b/src/pages/DynamicReportChangeWorkspacePage.tsx index 343c64c736c5..5d956e32743f 100644 --- a/src/pages/DynamicReportChangeWorkspacePage.tsx +++ b/src/pages/DynamicReportChangeWorkspacePage.tsx @@ -221,7 +221,6 @@ function DynamicReportChangeWorkspacePage({report}: DynamicReportChangeWorkspace currentUserLogin: session?.email, shouldShowPendingDeletePolicy: false, selectedPolicyIDs: currentSelection ? [currentSelection] : undefined, - // The report's saved workspace stays pinned to the top, so checking another row doesn't reorder the list before the user saves. policyIDsToSortToTop: report.policyID ? [report.policyID] : undefined, searchTerm: debouncedSearchTerm, localeCompare, diff --git a/src/pages/SetDefaultWorkspacePage.tsx b/src/pages/SetDefaultWorkspacePage.tsx index 9c20295dc2ba..1599734af2c7 100644 --- a/src/pages/SetDefaultWorkspacePage.tsx +++ b/src/pages/SetDefaultWorkspacePage.tsx @@ -75,7 +75,7 @@ function SetDefaultWorkspacePage({route}: SetDefaultWorkspacePageProps) { currentUserLogin: session?.email, shouldShowPendingDeletePolicy: false, selectedPolicyIDs: draftPolicyID ? [draftPolicyID] : undefined, - // This page never pinned a workspace to the top, so keep the plain alphabetical order instead of pinning the row the user just checked. + // This page never pinned a workspace to the top, so don't start now that checking a row sets selectedPolicyIDs. shouldSortSelectedToTop: false, searchTerm: debouncedSearchTerm, localeCompare, @@ -86,8 +86,7 @@ function SetDefaultWorkspacePage({route}: SetDefaultWorkspacePageProps) { showButton: true, text: translate('common.save'), onConfirm: () => selectPolicy(draftPolicyID), - // Unlike the other two pickers, selectPolicy also advances the flow (it navigates to navigateTo), so confirming the - // already-active workspace is a real action. Gate on whether a row is checked, not on whether the value changed. + // selectPolicy also advances the flow, so re-confirming the already-active workspace is a real action rather than a no-op. isDisabled: !draftPolicyID, }; diff --git a/src/pages/domain/Groups/BaseDomainGroupPreferredWorkspacePage.tsx b/src/pages/domain/Groups/BaseDomainGroupPreferredWorkspacePage.tsx index 27541b6f9ff9..3a0d1707efa0 100644 --- a/src/pages/domain/Groups/BaseDomainGroupPreferredWorkspacePage.tsx +++ b/src/pages/domain/Groups/BaseDomainGroupPreferredWorkspacePage.tsx @@ -1,8 +1,7 @@ /** * Shared preferred-workspace selector for domain groups. Both the group-create and group-edit * pages delegate to this component; it renders the admin workspace list with a search field and - * gates access behind DomainNotFoundPageWrapper. Callers choose how a pick commits: immediately on - * row press, or staged until Save via `shouldConfirmSelection`. + * gates access behind DomainNotFoundPageWrapper. */ import type {FullPageNotFoundViewProps} from '@components/BlockingViews/FullPageNotFoundView'; import HeaderWithBackButton from '@components/HeaderWithBackButton'; @@ -38,16 +37,13 @@ type BaseDomainGroupPreferredWorkspacePageProps = { /** AccountID of the domain */ domainAccountID: number; - /** The policy ID of the saved preferred workspace. It stays in the list even when the user is not its admin, and it is the row focused on open. */ + /** The policy ID of the saved preferred workspace */ selectedPolicyID: string | undefined; - /** Commits the picked workspace. Fired on row press, or on Save when `shouldConfirmSelection` is set. */ + /** Called with the policy ID of the workspace the user picked */ onSelectWorkspace: (policyID: string) => void; - /** - * Whether picking a row only stages the choice, leaving a Save button to commit it. Set it wherever committing on - * press would change the layout or navigate away, which WCAG 3.2.2 "On Input" disallows without an explicit confirm. - */ + /** Whether a pick is staged behind a Save button, which WCAG 3.2.2 "On Input" requires when committing navigates away */ shouldConfirmSelection?: boolean; /** Called when the back button is pressed */ @@ -77,8 +73,6 @@ function BaseDomainGroupPreferredWorkspacePage({ const {translate, localeCompare} = useLocalize(); const [draftPolicyID, setDraftPolicyID] = useState(); - - // The saved workspace drives which rows the list contains and which one opens focused, so staging a pick can't drop it or move the list under the user. const checkedPolicyID = draftPolicyID ?? selectedPolicyID; const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: createAdminPoliciesSelector(selectedPolicyID)}); From 72574f1d654b8d3b4dd781faa7e112a7297759ce Mon Sep 17 00:00:00 2001 From: "truph01 (via MelvinBot)" Date: Thu, 10 Sep 2026 03:27:06 +0000 Subject: [PATCH 6/6] Remove the redundant isDisabled comment on SetDefaultWorkspacePage The comment justified not comparing draftPolicyID against activePolicyID, but that comparison is already gone as of 80cc7fb. 'isDisabled: !draftPolicyID' reads plainly on its own. --- src/pages/SetDefaultWorkspacePage.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages/SetDefaultWorkspacePage.tsx b/src/pages/SetDefaultWorkspacePage.tsx index 1599734af2c7..385cf460bb9c 100644 --- a/src/pages/SetDefaultWorkspacePage.tsx +++ b/src/pages/SetDefaultWorkspacePage.tsx @@ -86,7 +86,6 @@ function SetDefaultWorkspacePage({route}: SetDefaultWorkspacePageProps) { showButton: true, text: translate('common.save'), onConfirm: () => selectPolicy(draftPolicyID), - // selectPolicy also advances the flow, so re-confirming the already-active workspace is a real action rather than a no-op. isDisabled: !draftPolicyID, };