Open m-natarajan opened 3 days ago
melvin-bot[bot]
commented
3 days ago Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
nkdengineer
commented
3 days ago The checkmark isn't displayed when back to access level page
Currently we are using the goBack function here to navigate when the user presses the back button
But before that route params in route were undefined so route.params.role here will be undefined
We should set value to role params before we navigate to confirm page
Navigation.setParams({
role: option.value,
});
Navigation.navigate(ROUTES.SETTINGS_DELEGATE_CONFIRM.getRoute(login, option.value));Or we can use navigate function instead of goBack function here
jayeshmangwani
commented
3 days ago The previously selected role isn't displayed as selected when navigating back to the Access Level page using the header back button from the Add Copilot page.
On the SelectDelegateRolePage, we rely on the role parameter from the route passed from the previous screen using this condition:isSelected: role === route.params.role.
However, when navigating back from the Add Copilot page using the header back button, the checkmark for the previously selected user isn’t displayed because the parameter isn’t passed in this case. On the other hand, if the "Access Level" item on the Add Copilot page is clicked, the pre-selection is displayed since the parameter is passed during navigation.
Instead of relying solely on the route.params.role, we can manage the selection locally using useState. For example:
const [currentRole, setCurrentRole] = useState(route.params.role);When generating roleOptions, compare the role with currentRole to determine the value of isSelected.
On row selection (onSelectRow), update the local state (setCurrentRole), and then invoke the existing Navigation.navigate method.
Replace Navigation.goBack() with explicit navigation that ensures the necessary parameters are passed. For example:
Navigation.navigate(
ROUTES.SETTINGS_DELEGATE_ROLE.getRoute(option?.login ?? '')
);
bernhardoj
commented
3 days ago The checkmark isn't displayed on the previously selected role.
We show the selected icon only if the route params role equals the role itself. https://github.com/Expensify/App/blob/4c44827398f884af626872c33f64dd358d19eff4/src/pages/settings/Security/AddDelegate/SelectDelegateRolePage.tsx#L24-L28
But when we open the select role for the first time, the role is undefined. As mentioned by the others, when we press back, we actually call goBack with a fallback of the select role page with the selected role passed to the param. https://github.com/Expensify/App/blob/4c44827398f884af626872c33f64dd358d19eff4/src/pages/settings/Security/AddDelegate/ConfirmDelegatePage.tsx#L54
But this just simply goes back, unless you refresh the page first. But the thing I want to focus on here is that I think we shouldn't show the selected icon in a new delegate flow (when we are going to select the role for the first time), just like when you select the delegate user, there is no selected icon. Because it's a single list selection, the selected icon feels like I'm editing the option.
But the reason QA expects the selected icon is maybe because of the highlighted background of the role. It's because we pass shouldUpdateFocusedIndex which updates the focused item to the pressed item.
https://github.com/Expensify/App/blob/4c44827398f884af626872c33f64dd358d19eff4/src/pages/settings/Security/AddDelegate/SelectDelegateRolePage.tsx#L45
If we agree with what I explain above, then we can remove shouldUpdateFocusedIndex (or set it to false when role is undefined)
https://github.com/Expensify/App/blob/4c44827398f884af626872c33f64dd358d19eff4/src/pages/settings/Security/AddDelegate/SelectDelegateRolePage.tsx#L45
And update the confirm role page to not pass the role when going back. https://github.com/Expensify/App/blob/4c44827398f884af626872c33f64dd358d19eff4/src/pages/settings/Security/AddDelegate/ConfirmDelegatePage.tsx#L53-L54
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 9.0.64-4 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @DylanDylann Slack conversation (hyperlinked to channel name): ts_external_expensify_bugs
Action Performed:
Expected Result:
The checkmark should be displayed when back to access level page for the access level selected
Actual Result:
The checkmark isn't displayed when back to access level page , selected level is highlighted
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/user-attachments/assets/dfcec016-4a00-42b3-b58a-2aa485b70613
https://github.com/user-attachments/assets/547e00ac-5766-41f9-b6b1-5a47135ec0a3
View all open jobs on GitHub