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

Added bouncing[IOS]/overScroll Edge[Android] effect to the lists of the app #5136

Merged
merged 7 commits into from
Sep 14, 2021

Conversation

parasharrajat
Copy link
Member

@parasharrajat parasharrajat commented Sep 8, 2021

Details

#4816 (comment)

Enabled pages:

  • LHN.
  • Participant list on IOU.
  • Search page list.
  • new chat list.
  • new group chat list.
  • Main Messages List of a chat.
  • Profile page and other pages which use Scrollview.
  • All FlatLists (including Emojipicker).
  • All SectionLists.

Fixed Issues

$ #4816

Tests

  1. Tested on Settings Page.
  2. Tested on the Split bill Participants list.

QA Steps

  1. Open New Expensify on Android or iOS
  2. Navigate to all the pages one by one which has a list of some kind or that page can be scrolled.
  3. Scroll to the top edge and scroll to the bottom edge respectively.
  4. You should see the effect.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

effect-opt-i.mp4

Android

output_file.mp4

@parasharrajat parasharrajat marked this pull request as ready for review September 10, 2021 19:19
@parasharrajat parasharrajat requested a review from a team as a code owner September 10, 2021 19:19
@MelvinBot MelvinBot requested review from chiragsalian and removed request for a team September 10, 2021 19:19
@parasharrajat parasharrajat changed the title added bouncing efffect to Lists of the app [WIP] Added bouncing efffect to Lists of the app Sep 10, 2021
@parasharrajat parasharrajat changed the title [WIP] Added bouncing efffect to Lists of the app Added bouncing efffect to Lists of the app Sep 13, 2021
@parasharrajat
Copy link
Member Author

@shawnborton I have updated the list of components in the description which are enabled for the effect. However, there are other components that I would like to confirm first.

  1. Main list of chat messages.
  2. LHN chat list. Currently active.
  3. Currency Selection page.
  4. IOU reports list.

Please let me know if there are any other lists that you want me to activate.

@parasharrajat parasharrajat changed the title Added bouncing efffect to Lists of the app Added bouncing[IOS]/overScroll Edge[Android] effect to the lists of the app Sep 13, 2021
@shawnborton
Copy link
Contributor

I think that sounds pretty good to me, I think basically any view that can scroll would have this behavior.

@parasharrajat
Copy link
Member Author

All set here. Ready for review @shawnborton @chiragsalian. Thanks for the patience.

@chiragsalian
Copy link
Contributor

I didn't get a chance to review today but i'll review it tomorrow.

chiragsalian
chiragsalian previously approved these changes Sep 14, 2021
Copy link
Contributor

@chiragsalian chiragsalian left a comment

Choose a reason for hiding this comment

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

Changes looks good to me. One question though, is there a reason bounce isn't added here. I briefly tested the terms page and the scroll did bounce even though the scrollView didn't have bounce mentioned so just wanted to confirm we're not missing something.

@parasharrajat
Copy link
Member Author

Oh, Thanks @chiragsalian for end moment catch. I didn't notice that bounces={true} is default. So I can just remove the bounces prop. Give me a minute. I will push a new commit.

@chiragsalian
Copy link
Contributor

Also, the terms page is the only one i noticed that has a scroll in the middle of the text which is pretty weird. Was wondering if you can resolve that here,
image

To test the terms page I followed the steps from this PR.

@parasharrajat
Copy link
Member Author

@chiragsalian Done.

Copy link
Contributor

@chiragsalian chiragsalian left a comment

Choose a reason for hiding this comment

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

Nice, thank you for the changes. Looks good to me and works well.

@chiragsalian chiragsalian merged commit 145eb96 into Expensify:main Sep 14, 2021
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @chiragsalian in version: 1.0.98-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @Jag96 in version: 1.0.99-4 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@parasharrajat parasharrajat deleted the bounce branch November 20, 2023 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants