Skip to content
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

[$250] Android - Reports - Bottom part of expenses list is cutted off after renaming a saved search #56156

Open
2 of 8 tasks
IuliiaHerets opened this issue Jan 31, 2025 · 15 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Jan 31, 2025

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: 9.0.93-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes, reproducible on both
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Device used: Motorola MotoG60 - Android 12
App Component: Search

Action Performed:

  1. Open the Expensify app.
  2. Tap on "Reports"
  3. If the expenses list is not scrollable yet, create expenses until making it scrollable. (Between 7 and 9 expenses)
  4. Tap on the filters icon.
  5. Apply any filter to trigger a search and save the search.
  6. Tap on the dropdown menu.
  7. Tap on the three dots icon of the saved search and tap on "Rename"
  8. Add any name to the search and save it.
  9. Tap on "Cancel" on top of the screen.
  10. Check the behaviour of the bottom part of expenses list, after this action.

Expected Result:

Expenses list should be fully visible after a saved search is rename and the filters are cleared.

Actual Result:

Bottom part of expenses list on "Reports" gets cutted of after renaming a saved search and tapping "Cancel" to clear all the filters. This issue reproduces when expenses list becomes scrollable and the user has between seven and nine expenses on the list.

Workaround:

Unknown

Platforms:

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6729485_1738322600127.Cutted.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021886841740639100643
  • Upwork Job ID: 1886841740639100643
  • Last Price Increase: 2025-02-11
Issue OwnerCurrent Issue Owner: @dukenv0307
@IuliiaHerets IuliiaHerets added Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 labels Jan 31, 2025
Copy link

melvin-bot bot commented Jan 31, 2025

Triggered auto assignment to @dylanexpensify (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.

@melvin-bot melvin-bot bot added the Overdue label Feb 3, 2025
Copy link

melvin-bot bot commented Feb 4, 2025

@dylanexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

@dylanexpensify dylanexpensify added the External Added to denote the issue can be worked on by a contributor label Feb 4, 2025
@melvin-bot melvin-bot bot changed the title Android - Reports - Bottom part of expenses list is cutted off after renaming a saved search [$250] Android - Reports - Bottom part of expenses list is cutted off after renaming a saved search Feb 4, 2025
Copy link

melvin-bot bot commented Feb 4, 2025

Job added to Upwork: https://www.upwork.com/jobs/~021886841740639100643

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 4, 2025
Copy link

melvin-bot bot commented Feb 4, 2025

Triggered auto assignment to Contributor-plus team member for initial proposal review - @dukenv0307 (External)

@melvin-bot melvin-bot bot removed the Overdue label Feb 4, 2025
@dukenv0307
Copy link
Contributor

@IuliiaHerets I can't reproduce on Android standalone, can you please try it on another device?

@IuliiaHerets
Copy link
Author

@dukenv0307 Two testers can still repro this issue. One of them noticed that the issue can be repro when the saved search list is a little bit scrollable

https://github.com/user-attachments/assets/19937513-93c1-4319-82a0-59bea159429c
https://github.com/user-attachments/assets/2e65eac7-23fc-4126-bb93-b3e8a507cdb8

@dylanexpensify
Copy link
Contributor

Looks like we keep this going

Copy link

melvin-bot bot commented Feb 10, 2025

@dylanexpensify, @dukenv0307 Huh... This is 4 days overdue. Who can take care of this?

@melvin-bot melvin-bot bot added the Overdue label Feb 10, 2025
@dukenv0307
Copy link
Contributor

Still waiting for proposal...

@melvin-bot melvin-bot bot removed the Overdue label Feb 11, 2025
Copy link

melvin-bot bot commented Feb 11, 2025

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@dylanexpensify
Copy link
Contributor

Same

@QichenZhu
Copy link
Contributor

QichenZhu commented Feb 13, 2025

Proposal

Please re-state the problem that we are trying to solve in this issue.

Reports screen is cut off after renaming a saved search.

What is the root cause of that problem?

There’s a short timing gap after KeyboardAvoidingView initializes and before it listens for keyboard changes.

When it initializes, the keyboard is open, so it sets heightWhenOpened, height, and progress to non-zero values (e.g., heightWhenOpened = 271, height = 271, progress = 1).

https://github.com/kirillzyusko/react-native-keyboard-controller/blob/54fa1eea43f3f9dd1c73ea9e9494e87d1c45d8e6/src/components/KeyboardAvoidingView/hooks.ts#L11-L17

const [initialHeight] = useState(() => -reanimated.height.value);
const [initialProgress] = useState(() => reanimated.progress.value);

const heightWhenOpened = useSharedValue(initialHeight);
const height = useSharedValue(initialHeight);
const progress = useSharedValue(initialProgress);

The keyboard then closes, and only after that KeyboardAvoidingView starts listening for keyboard changes, so it isn’t aware that the keyboard has already closed, and the keyboard change handlers shown below don't run.

https://github.com/kirillzyusko/react-native-keyboard-controller/blob/54fa1eea43f3f9dd1c73ea9e9494e87d1c45d8e6/src/components/KeyboardAvoidingView/hooks.ts#L19-L52

onMove: (e) => {
  "worklet";

  progress.value = e.progress;
  height.value = e.height;
},
onEnd: (e) => {
  "worklet";

  isClosed.value = e.height === 0;

  height.value = e.height;
  progress.value = e.progress;
},

heightWhenOpened, height, and progress are still at their initial values, then KeyboardAvoidingView sets the bottom padding using these incorrect values.

https://github.com/kirillzyusko/react-native-keyboard-controller/blob/54fa1eea43f3f9dd1c73ea9e9494e87d1c45d8e6/src/components/KeyboardAvoidingView/index.tsx#L117C1-L121C9

const relativeKeyboardHeight = useCallback(() => {
  "worklet";

  const keyboardY =
    screenHeight - keyboard.heightWhenOpened.value - keyboardVerticalOffset;

  return Math.max(frame.value.y + frame.value.height - keyboardY, 0);
}, [screenHeight, keyboardVerticalOffset]);

(keyboardY = 792 - 271 - 0 = 521, relativeKeyboardHeight() = max(31 + 689 - 521, 0) = 199)

const bottom = interpolate(
  keyboard.progress.value,
  [0, 1],
  [0, relativeKeyboardHeight()],
);

(keyboard.progress.value = 1, relativeKeyboardHeight() = 199, bottom = 199)

What changes do you think we should make in order to solve the problem?

Initialize values in an effect right before calling useKeyboardHandler().

const heightWhenOpened = useSharedValue(0);
const height = useSharedValue(0);
const progress = useSharedValue(0);
const isClosed = useSharedValue(true);

useEffect(() => {
  const initialHeight = -reanimated.height.value;
  const initialProgress = reanimated.progress.value;

  heightWhenOpened.value = initialHeight;
  height.value = initialHeight;
  progress.value = initialProgress;
  isClosed.value = initialProgress === 0;
}, []);

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

This needs manual testing.

What alternative solutions did you explore? (Optional)

Initialize values as the keyboard is closed. This approach is not recommended as it will bring back another issue: kirillzyusko/react-native-keyboard-controller#398. However, the issue only affects Android 10 or lower which Google no longer supports so it's not a big problem.

const heightWhenOpened = useSharedValue(0);
const height = useSharedValue(0);
const progress = useSharedValue(0);
const isClosed = useSharedValue(true);

@melvin-bot melvin-bot bot added the Overdue label Feb 13, 2025
@dukenv0307
Copy link
Contributor

Thanks for your proposal @QichenZhu

KeyboardAvoidingView misses the change, so it doesn't update height and progress, and sets the bottom padding based on the initial values.

I don't understand this point. Can you please elaborate on the RCA with a detailed flow, reference and the value at each stage?
Since this issue is hard to reproduce, you should share the video before and after the fix

@melvin-bot melvin-bot bot removed the Overdue label Feb 13, 2025
@QichenZhu
Copy link
Contributor

Thanks for your reivew. @dukenv0307

Can you please elaborate on the RCA with a detailed flow, reference and the value at each stage?

Proposal updated.

Since this issue is hard to reproduce, you should share the video before and after the fix.

I tested with production APKs built using npm run android-build. Timing is the key factor in this issue. It only happens if the keyboard closes after KeyboardAvoidingView initializes but before it binds to keyboard change handlers.

Before fix After fix
before.mp4
after.mp4

Copy link

melvin-bot bot commented Feb 14, 2025

@dylanexpensify @dukenv0307 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

4 participants