[$500] Android - Taxes - Cursor is placed before the value when reopening saved value
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.4.57-2 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team
Issue found when executing PR https://github.com/Expensify/App/pull/38733
Action Performed:
Precondition:
- User is admin of Collect workspace
- Go to staging.new.expensify.com
- Go to Profile > Workspaces > Collect workspace
- Go to Taxes > Add rate > Value
- Enter any value and save it
- Tap Value again
Expected Result:
Cursor is placed behind the value
Actual Result:
Cursor is placed in front of the value. Sometimes it appears behind the value
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [x] Android: Native
- [ ] Android: mWeb Chrome
- [ ] iOS: Native
- [ ] iOS: mWeb Safari
- [ ] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/78819774/92f43526-549a-43cf-8d7b-de7644b2ab7f
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~01515a93e0ea4bae26
- Upwork Job ID: 1774175994999578624
- Last Price Increase: 2024-04-13
Triggered auto assignment to @MitchExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
@MitchExpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors
We think that this bug might be related to #wave-control
Proposal
Please re-state the problem that we are trying to solve in this issue.
Android - Taxes - Cursor is placed before the value when reopening saved value
What is the root cause of that problem?
We are not updating the selection correctly and due to that sometimes the selection is set to 0,0. We should set selection exactly like we do in MoneyRequestAmountForm.
https://github.com/Expensify/App/blob/c7d692662d069ef8d9c756833767c47e1f7410a2/src/components/AmountForm.tsx#L220-L225
https://github.com/Expensify/App/blob/c7d692662d069ef8d9c756833767c47e1f7410a2/src/pages/iou/steps/MoneyRequestAmountForm.tsx#L323-L331
What changes do you think we should make in order to solve the problem?
We need to calculate the start and end of the selection instead of using e.nativeEvent.selection. Or we can refrain from updating selection on initial render. For that we can use a ref or state.
onSelectionChange={(e: NativeSyntheticEvent<TextInputSelectionChangeEventData>) => {
if (!shouldUpdateSelection) {
return;
}
const maxSelection = formattedAmount.length;
const start = Math.min(e.nativeEvent.selection.start, maxSelection);
const end = Math.min(e.nativeEvent.selection.end, maxSelection);
setSelection({start, end});
}}
What alternative solutions did you explore? (Optional)
We don't clear the amount when going back to Add rate page, we should also reset the value when the component AmountForm component mounts.
We need to add changes here: https://github.com/Expensify/App/blob/14ff9445d22bd92d0645abd456ac805cedc06c28/src/components/AmountPicker/AmountSelectorModal.tsx#L19
To:
useEffect(() => {
setValue(value);
}, [isVisible, value]);
Or we can update onBackButtonPress here like:
https://github.com/Expensify/App/blob/14ff9445d22bd92d0645abd456ac805cedc06c28/src/components/AmountPicker/AmountSelectorModal.tsx#L37
onBackButtonPress={() => {
onClose();
setValue(value);
}}
What alternative solutions did you explore? (Optional) 2
We should focus on the input using inputCallbackRef from the useAutoFocusInput hook. We need to utilize the ref passed from AmountForm to AmountSelectorModal and then apply inputCallbackRef.
Or we can follow the code we use in IOURequestStepAmount:
https://github.com/Expensify/App/blob/e542ba7c2d9e216c2cd401de0de8ed225fff4a7b/src/pages/iou/request/step/IOURequestStepAmount.js#L87-L97
If we want to fix this issue comprehensively, we can check for the autoFocus prop and replace it with the implementation mentioned above. Additionally, we can cease accepting the autoFocus prop altogether to prevent encountering this issue again in the future.
pseudo-code:
const textInput = useRef<BaseTextInputRef | null>(null);
const focusTimeoutRef = useRef<NodeJS.Timeout | null>(null);
useFocusEffect(
useCallback(() => {
if (!isVisible) {
return;
}
focusTimeoutRef.current = setTimeout(() => textInput.current && textInput.current.focus(), CONST.ANIMATED_TRANSITION);
return () => {
if (!focusTimeoutRef.current) {
return;
}
clearTimeout(focusTimeoutRef.current);
};
}, [isVisible]),
);
Job added to Upwork: https://www.upwork.com/jobs/~01515a93e0ea4bae26
Triggered auto assignment to Contributor-plus team member for initial proposal review - @cubuspl42 (External)
Not reproducible on main in my case:
https://github.com/Expensify/App/assets/59809993/d3da0e06-f5a5-4a53-9311-712ca89c26aa
Proposals ready for you @cubuspl42
Sorry if this is obvious, but how can I enter the "Taxes" screen?
This is a Collect workspace, I'm the admin.
@cubuspl42 below Profile and Members you should also see More Features. Enable Taxes under More Features. My collect workspace was created on old dot. I don't know if that makes a difference.
@cubuspl42, I guess you are not the admin of the workspace.
I thought "owner" and "admin" are the same. You can see that I'm the owner on the screenshots.
It should have all these options.
I found a solution; see this Slack message. I'll see if I can reproduce this issue now.
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@lanitochka17 @MitchExpensify I can't reproduce, same as in this comment.. I tested on latest main.
@cubuspl42, I can reproduce on physical device when opening amount page multiple times and sometimes I had to only visit once or twice. Also, what do you think about the other bug? The amount is not cleared even if it is not saved.
https://github.com/Expensify/App/assets/85894871/a9b7734a-fcb7-4ca1-a8b7-b1203f0a5866
cc: @MitchExpensify
@Krishna2323 Actually, before we resume discussing your proposal, could you stick to the proposal template and fix the markdown of your proposal? Currently, I can see something like this:
First, "Result" and "Optional" aren't valid H3 headers from the proposal template, second, "Result" doesn't seem to render. Possibly, the same information could be provided in a H4 header, or in a separate GH comment.
@cubuspl42, thanks for the feedback, proposal template updated.
@cubuspl42 More difficult to reproduce it now, but it exists. Need to go to the page many times now.
https://github.com/Expensify/App/assets/78819774/1ac0914e-469e-4acc-addd-88431d5ded3f
Waiting on @cubuspl42 to check out the latest updated proposal
I'll run the app on my old physical Galaxy S8 to see if I can reproduce. It will help during testing, but as a plan B we could go forward without me reproducing, as we have double confirmation that the bug still occurs.
@cubuspl42 @MitchExpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
There are some building issues on Android, but I'll try working it around.
This fix for Android is now merged, I'm now building the app again