Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
4.03k stars 3.03k forks source link

[$250] Search - the search circle is not fully visible #56375

Closed lanitochka17 closed 1 week ago

lanitochka17 commented 1 week ago

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.94-1 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: 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

Action Performed:

  1. Open app in both mweb and android
  2. Tap search
  3. Enter a email id and tap a space

Expected Result:

In mweb and android, spinning circle must be fully visible

Actual Result:

In mweb, spinning circle is fully visible but in android , circle is not fully visible

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/ca8547b2-5b88-4e9b-8416-87242110480d

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021887242265582398778
  • Upwork Job ID: 1887242265582398778
  • Last Price Increase: 2025-02-05
Issue OwnerCurrent Issue Owner: @jayeshmangwani
melvin-bot[bot] commented 1 week ago

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

truph01 commented 1 week ago

Proposal

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

What is the root cause of that problem?

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

NA

What alternative solutions did you explore? (Optional)

NA

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

kadiealexander commented 1 week ago

iOS <> Android bug swap with @jliexpensify

jliexpensify commented 1 week ago

Can reproduce, search circle is barely there for me (Pixel 7)

huult commented 1 week ago

Proposal

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

the search circle is not fully visible

What is the root cause of that problem?

mWeb uses consistent browser-based CSS animations, while Android Native relies on ActivityIndicator, which is platform-dependent and affected by device settings. This causes rendering inconsistencies like partial visibility on Android.

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

To resolve this issue, we should replace ActivityIndicator with Lottie for loading. To implement this, we need the following steps:

1 Need design json loading for Lottie 2 Create new component loading Lottie 3 Replace here and here

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

None

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

jayeshmangwani commented 1 week ago

Here for search, we are using React Native's default ActivityIndicator, which differs between iOS and Android—it displays the respective OS's circular loading indicator.

If we want to change this and use a unified custom loader, I think we should confirm with the design team first.

@Expensify/design, what do you think about this issue? Should we update the search loader to ensure consistency across platforms, or does the current loader look fine?

I’m attaching videos for Android, iOS, and mWeb below for comparison. Please let us know your thoughts!

mWeb chrome https://github.com/user-attachments/assets/16efb64a-ad14-427a-b3c3-47689fe06b82
iOS https://github.com/user-attachments/assets/b3511f01-9da9-4840-8986-0f6439385750
Android https://github.com/user-attachments/assets/fdfb67fd-f3e4-4696-acf3-dde45d7d7599
dannymcclain commented 1 week ago

Here for search, we are using React Native's default ActivityIndicator, which differs between iOS and Android—it displays the respective OS's circular loading indicator.

As far as I know, we use this same loader for all our spinners across the app. I feel like this has come up a few times in the past and we've always decided to just continue using the default ActivityIndicator. I don't feel strongly about changing that decision now. @Expensify/design how about you all?

shawnborton commented 1 week ago

Yeah I think it's fine to just keep using what we currently have as well, and then later on we can follow up and implement a custom loading spinner everywhere.

dubielzyk-expensify commented 1 week ago

Yeah I agree with both of you 👍

jliexpensify commented 1 week ago

Cool, ty @Expensify/design team - so close this one out then?

shawnborton commented 1 week ago

I would say so, yeah.

jliexpensify commented 1 week ago

Thanks, job closed!