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

feat: activity centre animations #15262

Closed
wants to merge 4 commits into from
Closed

Conversation

OmarBasem
Copy link
Contributor

@OmarBasem OmarBasem commented Mar 6, 2023

fixes: #15261

This PR migrates activity centre away from using the popover component to using Modal component from React Native, plus some minor changes.

This PR introduces the following improvements:

  • Reduces AC opening delay from being about 100ms to instantly.
  • Improves the animation smoothness
  • Removes that weird background overlay that suddenly appears before the animation, and suddenly disappears after the animation
  • Fixes Status bar coloring on Android. For iOS see issue: Updating status bar using merge options not working on iOS #15260
  • Makes the Status bar on Android translucent, to reflect the designs more accurately

And here how it looks:

-> on iOS:

Simulator.Screen.Recording.-.iPhone.13.Pro.-.2023-03-06.at.14.10.47.mp4

-> and Android:

Screen_Recording_20230306-092344_Status.PR_1.mp4

@status-im-auto
Copy link
Member

status-im-auto commented Mar 6, 2023

Jenkins Builds

Click to see older builds (8)
Commit #️⃣ Finished (UTC) Duration Platform Result
6f41b33 #1 2023-03-06 07:34:21 ~2 min tests 📄log
✔️ 6f41b33 #1 2023-03-06 07:41:13 ~9 min android 🤖apk 📲
✔️ 6f41b33 #1 2023-03-06 07:41:18 ~9 min android-e2e 🤖apk 📲
✔️ 6f41b33 #1 2023-03-06 07:41:39 ~9 min ios 📱ipa 📲
08492da #2 2023-03-06 07:52:32 ~3 min tests 📄log
✔️ 08492da #2 2023-03-06 07:55:49 ~7 min ios 📱ipa 📲
✔️ 08492da #2 2023-03-06 07:56:27 ~7 min android 🤖apk 📲
✔️ 08492da #2 2023-03-06 07:56:29 ~7 min android-e2e 🤖apk 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
1942a32 #3 2023-03-06 10:02:59 ~3 min tests 📄log
✔️ 1942a32 #3 2023-03-06 10:09:16 ~9 min android 🤖apk 📲
✔️ 1942a32 #3 2023-03-06 10:09:17 ~9 min android-e2e 🤖apk 📲
✔️ 1942a32 #3 2023-03-06 10:11:20 ~11 min ios 📱ipa 📲
dd7c8b9 #4 2023-03-06 10:14:44 ~2 min tests 📄log
✔️ dd7c8b9 #4 2023-03-06 10:19:16 ~7 min ios 📱ipa 📲
✔️ dd7c8b9 #4 2023-03-06 10:19:49 ~7 min android-e2e 🤖apk 📲
✔️ dd7c8b9 #4 2023-03-06 10:20:53 ~8 min android 🤖apk 📲

Copy link
Member

@flexsurfer flexsurfer left a comment

Choose a reason for hiding this comment

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

why not RNN modal ?

@OmarBasem
Copy link
Contributor Author

why not RNN modal ?

@flexsurfer The modal from React Native is smoother. RNN only has a modal for Android, and it seems to be animating on the JS thread. For iOS, they have no modal, and they recommend using modal from React Native.

@flexsurfer
Copy link
Member

why not RNN modal ?

@flexsurfer The modal from React Native is smoother. RNN only has a modal for Android, and it seems to be animating on the JS thread. For iOS, they have no modal, and they recommend using modal from React Native.

that's weird, the whole point of RNN is that it's written in native code, we need to double check

@flexsurfer
Copy link
Member

why not RNN modal ?

@flexsurfer The modal from React Native is smoother. RNN only has a modal for Android, and it seems to be animating on the JS thread. For iOS, they have no modal, and they recommend using modal from React Native.

image

if you mean this its only for declaring modal as a Component inside render function we don't need this

@flexsurfer
Copy link
Member

RNN should be more performant, because modal is native, separate native container, in the RN its just view inside react tree, so it will be slower for sure, but that's my guess

@flexsurfer
Copy link
Member

we have :open-modal event , try it

@flexsurfer
Copy link
Member

flexsurfer commented Mar 6, 2023

we've never had any issues with it, we have lots of modals

@OmarBasem
Copy link
Contributor Author

OmarBasem commented Mar 6, 2023

Hey @flexsurfer, I think I wasn't clear. I am talking about "Modal component" not "Modal screen". Unfortunately, using a screen is it not possible for the transparent blurry background design. So we are using a Modal component.

The Modal component from React Native on iOS is purely native. On Android, they have their own native implementation, at least for the animations (as the Android OS does not actually have a native Modal component that is not a screen). It looks perfect on iOS, and looks good on Android.

I was hoping RNN modal to look better on Android, but unfortunately it does not. I also tried checking other modal libs: https://github.com/jeremybarbet/react-native-modalize, https://gorhom.github.io/react-native-bottom-sheet/
For a full screen Modal "component", the one from React Native looks best.

@flexsurfer
Copy link
Member

from your videos, i don't see the point of having a transparent screen, if the design team really wants a blur effect, can we just use a fake blur image as a background? btw on ios demo there is no blur

@OmarBasem
Copy link
Contributor Author

OmarBasem commented Mar 6, 2023

@flexsurfer I don't like the transparent background either. I have suggested to @ilmotta to have a fake background before #14968 (comment) but he is saying he chatted with the design team and John before over this, and they are not accepting design changes regarding that background.

My guess is that they want the activity center to look similar to the notifications center on iOS and Android, as we are supposed to be building a "super app" that looks like an Operating System.

There is a blur on the iOS demo, maybe I need to reduce it a little on iOS.

@flexsurfer
Copy link
Member

Unfortunately, using a screen is it not possible for the transparent blurry background design.

could you elaborate please

@OmarBasem
Copy link
Contributor Author

Unfortunately, using a screen is it not possible for the transparent blurry background design.

could you elaborate please

@flexsurfer setting the background color as transparent to a screen will just be white

updates

updates

updates

updates

updates

updates

updates

updates
@OmarBasem OmarBasem force-pushed the activity-centre-animations branch from 08492da to 1942a32 Compare March 6, 2023 09:59
@flexsurfer
Copy link
Member

Unfortunately, using a screen is it not possible for the transparent blurry background design.

could you elaborate please

@flexsurfer setting the background color as transparent to a screen will just be white

fixed #15268

@OmarBasem OmarBasem closed this Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Migrate Activity Centre away from Popover component
3 participants