Skip to content

Make NetSuiteCustomFieldMappingPicker use new SelectionList #72740

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.

Sign up for GitHub

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

Merged
grgia merged 6 commits into from
Oct 31, 2025
Merged

Make NetSuiteCustomFieldMappingPicker use new SelectionList #72740

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7939340
Make NetSuiteCustomFieldMappingPicker use new SelectionList
OlGierd03 Oct 16, 2025
d1e857a
Merge branch 'main' into @OlGierd03/migrate-NetSuiteCustomFieldMappin…
OlGierd03 Oct 16, 2025
d00c003
Fix the bug with navigating back to ROUTES.POLICY_ACCOUNTING_NETSUITE…
OlGierd03 Oct 16, 2025
69efb44
Delete unnecessary shouldUpdateFocusedIndex prop from SelectionList i…
OlGierd03 Oct 16, 2025
bc6f125
Merge branch 'main' into @OlGierd03/migrate-NetSuiteCustomFieldMappin…
OlGierd03 Oct 24, 2025
4df870f
Bring back the shouldUpdateFocusedIndex prop
OlGierd03 Oct 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/pages/workspace/accounting/netsuite/import/NetSuiteImportCustomFieldEdit.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function NetSuiteImportCustomFieldEdit({
}
}

Navigation.navigate(ROUTES.POLICY_ACCOUNTING_NETSUITE_IMPORT_CUSTOM_FIELD_VIEW.getRoute(policyID, importCustomField, valueIndex));
Navigation.goBack(ROUTES.POLICY_ACCOUNTING_NETSUITE_IMPORT_CUSTOM_FIELD_VIEW.getRoute(policyID, importCustomField, valueIndex));

@dukenv0307 dukenv0307 Oct 27, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it related to this PR?

@OlGierd03 OlGierd03 Oct 28, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is. It's a fix for a navigation issue that occurred after item selection from the migrated component. Previously, instead of closing the modal screen after the selection, the user was navigated to a new one, as visible in the video:

Screen.Recording.2025-10-28.at.12.43.02.mov

},
[allRecords, customField, fieldName, importCustomField, policyID, valueIndex],
);
Expand Down
8 changes: 4 additions & 4 deletions ...ounting/netsuite/import/NetSuiteImportCustomFieldNew/NetSuiteCustomFieldMappingPicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import React from 'react';
import {View} from 'react-native';
import FormHelpMessage from '@components/FormHelpMessage';
import SelectionList from '@components/SelectionListWithSections';
import RadioListItem from '@components/SelectionListWithSections/RadioListItem';
import SelectionList from '@components/SelectionList';
import RadioListItem from '@components/SelectionList/ListItem/RadioListItem';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -36,12 +36,12 @@ function NetSuiteCustomFieldMappingPicker({value, errorText, onInputChange}: Net
return (
<>
<SelectionList
sections={[{data: selectionData}]}
data={selectionData}
onSelectRow={(selected) => {
onInputChange?.(selected.value);
}}
ListItem={RadioListItem}
initiallyFocusedOptionKey={value ?? CONST.INTEGRATION_ENTITY_MAP_TYPES.TAG}
initiallyFocusedItemKey={value ?? CONST.INTEGRATION_ENTITY_MAP_TYPES.TAG}
shouldSingleExecuteRowSelect
shouldUpdateFocusedIndex

@dukenv0307 dukenv0307 Oct 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove shouldUpdateFocusedIndex here? Are there any cases where the user selects an item but the selection page is not closed?

@OlGierd03 OlGierd03 Oct 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This prop is not needed. Selecting an item in the component makes the SelectionList close in every use case

@dukenv0307 dukenv0307 Oct 25, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing shouldUpdateFocusedIndex can cause a regression like #73449. Let me test it more carefully. Or to be safe, we can bring it back.

@OlGierd03 OlGierd03 Oct 25, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alike regression shouldn't happen as the page always closes after an item selection. In the component that migration caused the issue (the one you mentioned), it turned out not to be the case. If you have any doubts, I would appreciate you testing it in detail

/>
Expand Down
Loading